Compare commits
27 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
 claw
|
90959da830 | ||
 aweiker
|
6be5e306aa | ||
|
claw
|
cd6cd93bf0 | ||
|
claw
|
c889724dda | ||
|
claw
|
1ac51669ed | ||
|
claw
|
2e6f46f28d | ||
 Rodin
|
3fc31c0822 | ||
|
Rodin
|
2b611dbd0b | ||
|
Rodin
|
3abb611baf | ||
|
Rodin
|
dd003c66d5 | ||
|
aweiker
|
019b815280 | ||
|
Rodin
|
c27dfd0f08 | ||
|
Rodin
|
1b6c37605f | ||
|
aweiker
|
036e96d9b7 | ||
|
Rodin
|
ea74f7e088 | ||
|
Rodin
|
e6b1840ffc | ||
|
aweiker
|
1ca9250e4a | ||
|
Rodin
|
1b38e6ad00 | ||
|
Rodin
|
5498dccd60 | ||
 Aaron Weiker
|
ecfbfddc7c | ||
|
Rodin
|
ac53ecfa5d | ||
|
Rodin
|
090ae3848c | ||
|
Rodin
|
23da7eedf5 | ||
|
Rodin
|
7279cdd216 | ||
|
aweiker
|
4c327b61d4 | ||
|
aweiker
|
877dbf9999 | ||
|
Rodin
|
4a1cb6b47c |
@@ -38,6 +38,8 @@ jobs:
|
||||
- name: security
|
||||
token_secret: SECURITY_REVIEW_TOKEN
|
||||
model: gpt-5
|
||||
patterns_repo: rodin/security-patterns
|
||||
patterns_files: "."
|
||||
system_prompt_file: SECURITY_REVIEW.md
|
||||
steps:
|
||||
- uses: actions/checkout@v4
|
||||
@@ -60,8 +62,8 @@ jobs:
|
||||
AICORE_API_URL: ${{ secrets.AICORE_API_URL }}
|
||||
AICORE_RESOURCE_GROUP: ${{ secrets.AICORE_RESOURCE_GROUP }}
|
||||
CONVENTIONS_FILE: "CONVENTIONS.md"
|
||||
PATTERNS_REPO: "rodin/go-patterns"
|
||||
PATTERNS_FILES: "README.md,patterns/"
|
||||
PATTERNS_REPO: ${{ matrix.patterns_repo || 'rodin/go-patterns' }}
|
||||
PATTERNS_FILES: ${{ matrix.patterns_files || 'README.md,patterns/' }}
|
||||
LLM_TIMEOUT: "600"
|
||||
SYSTEM_PROMPT_FILE: ${{ matrix.system_prompt_file }}
|
||||
run: ./review-bot
|
||||
|
||||
@@ -0,0 +1,200 @@
|
||||
# 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 (not required for aicore provider)'
|
||||
required: false
|
||||
default: ''
|
||||
llm-api-key:
|
||||
description: 'LLM API key (not required for aicore provider)'
|
||||
required: false
|
||||
default: ''
|
||||
llm-model:
|
||||
description: 'LLM model name'
|
||||
required: true
|
||||
llm-provider:
|
||||
description: 'LLM API provider: openai, anthropic, or aicore (default openai)'
|
||||
required: false
|
||||
default: 'openai'
|
||||
aicore-client-id:
|
||||
description: 'SAP AI Core client ID (required for aicore provider)'
|
||||
required: false
|
||||
default: ''
|
||||
aicore-client-secret:
|
||||
description: 'SAP AI Core client secret (required for aicore provider)'
|
||||
required: false
|
||||
default: ''
|
||||
aicore-auth-url:
|
||||
description: 'SAP AI Core authentication URL (required for aicore provider)'
|
||||
required: false
|
||||
default: ''
|
||||
aicore-api-url:
|
||||
description: 'SAP AI Core API URL (required for aicore provider)'
|
||||
required: false
|
||||
default: ''
|
||||
aicore-resource-group:
|
||||
description: 'SAP AI Core resource group (default: default)'
|
||||
required: false
|
||||
default: 'default'
|
||||
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: ''
|
||||
persona:
|
||||
description: 'Built-in persona name (security, architect, docs)'
|
||||
required: false
|
||||
default: ''
|
||||
persona-file:
|
||||
description: 'Path to custom persona JSON file'
|
||||
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:
|
||||
GITHUB_SERVER_URL: ${{ inputs.gitea-url || github.server_url }}
|
||||
GITHUB_REPOSITORY: ${{ 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 }}
|
||||
PERSONA: ${{ inputs.persona }}
|
||||
PERSONA_FILE: ${{ inputs.persona-file }}
|
||||
AICORE_CLIENT_ID: ${{ inputs.aicore-client-id }}
|
||||
AICORE_CLIENT_SECRET: ${{ inputs.aicore-client-secret }}
|
||||
AICORE_AUTH_URL: ${{ inputs.aicore-auth-url }}
|
||||
AICORE_API_URL: ${{ inputs.aicore-api-url }}
|
||||
AICORE_RESOURCE_GROUP: ${{ inputs.aicore-resource-group }}
|
||||
run: |
|
||||
ARGS=""
|
||||
if [ "${{ inputs.dry-run }}" = "true" ]; then
|
||||
ARGS="--dry-run"
|
||||
fi
|
||||
${{ runner.temp }}/review-bot $ARGS
|
||||
@@ -0,0 +1,69 @@
|
||||
name: CI
|
||||
|
||||
on:
|
||||
push:
|
||||
branches: [main]
|
||||
pull_request:
|
||||
types: [opened, synchronize]
|
||||
|
||||
jobs:
|
||||
test:
|
||||
runs-on: ubuntu-24.04
|
||||
steps:
|
||||
- uses: actions/checkout@v4
|
||||
- uses: actions/setup-go@v5
|
||||
with:
|
||||
go-version: '1.26'
|
||||
- run: go test ./...
|
||||
- run: go vet ./...
|
||||
- run: go build -o review-bot ./cmd/review-bot
|
||||
|
||||
# Self-review using native SAP AI Core provider
|
||||
# Models must match SAP AI Core deployments
|
||||
# Available models: gpt-5, anthropic--claude-4.6-sonnet, anthropic--claude-4.6-opus
|
||||
# Removed gpt-4.1, gpt-5-mini, gpt-4.1-mini - not deployed on AI Core
|
||||
review:
|
||||
runs-on: ubuntu-24.04
|
||||
if: github.event_name == 'pull_request'
|
||||
needs: test
|
||||
strategy:
|
||||
matrix:
|
||||
include:
|
||||
- name: sonnet
|
||||
token_secret: SONNET_REVIEW_TOKEN
|
||||
model: anthropic--claude-4.6-sonnet
|
||||
- name: gpt
|
||||
token_secret: GPT_REVIEW_TOKEN
|
||||
model: gpt-5
|
||||
- name: security
|
||||
token_secret: SECURITY_REVIEW_TOKEN
|
||||
model: gpt-5
|
||||
patterns_repo: rodin/security-patterns
|
||||
patterns_files: "."
|
||||
system_prompt_file: SECURITY_REVIEW.md
|
||||
steps:
|
||||
- uses: actions/checkout@v4
|
||||
- uses: actions/setup-go@v5
|
||||
with:
|
||||
go-version: '1.26'
|
||||
- run: go build -o review-bot ./cmd/review-bot
|
||||
- name: Run ${{ matrix.name }} review
|
||||
env:
|
||||
GITHUB_SERVER_URL: ${{ github.server_url }}
|
||||
GITHUB_REPOSITORY: ${{ github.repository }}
|
||||
PR_NUMBER: ${{ github.event.pull_request.number }}
|
||||
REVIEWER_TOKEN: ${{ secrets[matrix.token_secret] }}
|
||||
REVIEWER_NAME: ${{ matrix.name }}
|
||||
LLM_PROVIDER: aicore
|
||||
LLM_MODEL: ${{ matrix.model }}
|
||||
AICORE_CLIENT_ID: ${{ secrets.AICORE_CLIENT_ID }}
|
||||
AICORE_CLIENT_SECRET: ${{ secrets.AICORE_CLIENT_SECRET }}
|
||||
AICORE_AUTH_URL: ${{ secrets.AICORE_AUTH_URL }}
|
||||
AICORE_API_URL: ${{ secrets.AICORE_API_URL }}
|
||||
AICORE_RESOURCE_GROUP: ${{ secrets.AICORE_RESOURCE_GROUP }}
|
||||
CONVENTIONS_FILE: "CONVENTIONS.md"
|
||||
PATTERNS_REPO: ${{ matrix.patterns_repo || 'rodin/go-patterns' }}
|
||||
PATTERNS_FILES: ${{ matrix.patterns_files || 'README.md,patterns/' }}
|
||||
LLM_TIMEOUT: "600"
|
||||
SYSTEM_PROMPT_FILE: ${{ matrix.system_prompt_file }}
|
||||
run: ./review-bot
|
||||
@@ -0,0 +1,38 @@
|
||||
name: PR Ready Gate
|
||||
|
||||
on:
|
||||
pull_request:
|
||||
types: [synchronize]
|
||||
|
||||
jobs:
|
||||
clear-labels:
|
||||
runs-on: ubuntu-24.04
|
||||
# Always run - curl commands are safe if labels don't exist
|
||||
steps:
|
||||
- name: Remove ready and self-reviewed labels, reassign to author
|
||||
env:
|
||||
GITEA_TOKEN: ${{ secrets.RODIN_TOKEN }}
|
||||
run: |
|
||||
PR_NUMBER=${{ github.event.pull_request.number }}
|
||||
AUTHOR=${{ github.event.pull_request.user.login }}
|
||||
READY_LABEL_ID=38
|
||||
SELF_REVIEWED_LABEL_ID=37
|
||||
|
||||
# Remove ready label if present
|
||||
curl -sS -X DELETE \
|
||||
-H "Authorization: token $GITEA_TOKEN" \
|
||||
"https://gitea.weiker.me/api/v1/repos/${{ github.repository }}/issues/${PR_NUMBER}/labels/${READY_LABEL_ID}" || true
|
||||
|
||||
# Remove self-reviewed label if present
|
||||
curl -sS -X DELETE \
|
||||
-H "Authorization: token $GITEA_TOKEN" \
|
||||
"https://gitea.weiker.me/api/v1/repos/${{ github.repository }}/issues/${PR_NUMBER}/labels/${SELF_REVIEWED_LABEL_ID}" || true
|
||||
|
||||
# Reassign to author
|
||||
curl -sS -X PATCH \
|
||||
-H "Authorization: token $GITEA_TOKEN" \
|
||||
-H "Content-Type: application/json" \
|
||||
-d "{\"assignees\": [\"${AUTHOR}\"]}" \
|
||||
"https://gitea.weiker.me/api/v1/repos/${{ github.repository }}/pulls/${PR_NUMBER}"
|
||||
|
||||
echo "Cleared ready/self-reviewed labels and reassigned PR #${PR_NUMBER} to ${AUTHOR}"
|
||||
@@ -0,0 +1,97 @@
|
||||
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"
|
||||
@@ -329,11 +329,12 @@ All flags have environment variable equivalents:
|
||||
### Token Scopes Required
|
||||
|
||||
| Scope | Purpose |
|
||||
|-------|---------|
|
||||
|-------|--------|
|
||||
| `write:issue` | Post and delete reviews |
|
||||
| `write:repository` | Read PR diffs, file content, commit statuses |
|
||||
| `read:user` | Self-request as reviewer (optional but recommended) |
|
||||
|
||||
No `read:user` scope needed — the bot identifies itself from the review response.
|
||||
Without `read:user`, the bot still works but cannot add itself to the PR's reviewer list.
|
||||
|
||||
## Development
|
||||
|
||||
|
||||
@@ -54,8 +54,8 @@ func main() {
|
||||
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
|
||||
giteaURL := flag.String("gitea-url", envOrDefault("GITEA_URL", ""), "Gitea instance URL")
|
||||
repo := flag.String("repo", envOrDefault("GITEA_REPO", ""), "Repository (owner/name)")
|
||||
giteaURL := flag.String("gitea-url", envOrDefault("GITEA_URL", envOrDefault("GITHUB_SERVER_URL", "")), "Gitea instance URL")
|
||||
repo := flag.String("repo", envOrDefault("GITEA_REPO", envOrDefault("GITHUB_REPOSITORY", "")), "Repository (owner/name)")
|
||||
prNum := flag.String("pr", envOrDefault("PR_NUMBER", ""), "Pull request number")
|
||||
reviewerName := flag.String("reviewer-name", envOrDefault("REVIEWER_NAME", ""), "Reviewer display name")
|
||||
reviewerToken := flag.String("reviewer-token", envOrDefault("REVIEWER_TOKEN", ""), "Gitea token for posting review")
|
||||
|
||||
@@ -0,0 +1,268 @@
|
||||
# GitHub Support for review-bot
|
||||
|
||||
## Goal
|
||||
|
||||
AI code reviews on GitHub PRs using SAP AI Core as the LLM provider.
|
||||
|
||||
## Non-Goals
|
||||
|
||||
- Auto-detection of platform (explicit `--provider` flag is fine)
|
||||
- Unifying into one abstraction layer for its own sake
|
||||
|
||||
## Constraints
|
||||
|
||||
1. **Same features on both platforms** — anything review-bot does on Gitea should work on GitHub
|
||||
2. **Testable** — small interfaces, dependency injection, no global state
|
||||
3. **Interface from working code** — extract from gitea/, don't invent in vacuum
|
||||
|
||||
---
|
||||
|
||||
## Part 1: Feature Inventory
|
||||
|
||||
What does review-bot actually do?
|
||||
|
||||
### Core Review Flow
|
||||
|
||||
| Feature | Description |
|
||||
|---------|-------------|
|
||||
| Get PR metadata | Title, body, head SHA, base ref |
|
||||
| Get PR diff | Unified diff format |
|
||||
| Get PR files | List of changed files with status |
|
||||
| Get file content | Raw file at ref |
|
||||
| List directory | Enumerate files in path |
|
||||
| Post review | Body + inline comments + verdict |
|
||||
|
||||
### Review Management
|
||||
|
||||
| Feature | Description |
|
||||
|---------|-------------|
|
||||
| List reviews | Get existing reviews on PR |
|
||||
| Delete review | Remove old review before re-posting |
|
||||
| Get authenticated user | Who am I? |
|
||||
|
||||
### Platform-Specific (not in shared interface)
|
||||
|
||||
| Feature | Gitea | GitHub |
|
||||
|---------|-------|--------|
|
||||
| Resolve comment | Yes | No equivalent |
|
||||
| Timeline API | Yes | No equivalent |
|
||||
|
||||
These stay on gitea.Client directly. Callers that need them type-assert.
|
||||
|
||||
---
|
||||
|
||||
## Part 2: GitHub API Mapping
|
||||
|
||||
| Feature | Gitea API | GitHub API |
|
||||
|---------|-----------|------------|
|
||||
| Get PR | `GET /api/v1/repos/.../pulls/{n}` | `GET /repos/.../pulls/{n}` |
|
||||
| Get diff | `.diff` suffix | `Accept: application/vnd.github.diff` header |
|
||||
| Get files | `GET .../pulls/{n}/files` | Same |
|
||||
| Get file content | `GET .../raw/{path}?ref=` | `GET .../contents/{path}?ref=` + base64 decode |
|
||||
| List directory | `GET .../contents/{path}` | Same |
|
||||
| Post review | `POST .../pulls/{n}/reviews` | Same (adapter handles comment schema) |
|
||||
| List reviews | `GET .../pulls/{n}/reviews` | Same |
|
||||
| Delete review | `DELETE .../pulls/{n}/reviews/{id}` | Same |
|
||||
| Get user | `GET /api/v1/user` | `GET /user` |
|
||||
|
||||
---
|
||||
|
||||
## Part 3: Interface Design
|
||||
|
||||
**Principle:** Extract from working gitea/ code. The interface is discovered, not invented.
|
||||
|
||||
### Small, role-based interfaces
|
||||
|
||||
```go
|
||||
// vcs/interfaces.go
|
||||
|
||||
type PRReader interface {
|
||||
GetPullRequest(ctx context.Context, owner, repo string, number int) (*PullRequest, error)
|
||||
GetPullRequestDiff(ctx context.Context, owner, repo string, number int) (string, error)
|
||||
GetPullRequestFiles(ctx context.Context, owner, repo string, number int) ([]ChangedFile, error)
|
||||
}
|
||||
|
||||
type FileReader interface {
|
||||
GetFileContent(ctx context.Context, owner, repo, path, ref string) (string, error)
|
||||
ListContents(ctx context.Context, owner, repo, path string) ([]ContentEntry, error)
|
||||
}
|
||||
|
||||
type Reviewer interface {
|
||||
PostReview(ctx context.Context, owner, repo string, number int, req ReviewRequest) (*Review, error)
|
||||
ListReviews(ctx context.Context, owner, repo string, number int) ([]Review, error)
|
||||
DeleteReview(ctx context.Context, owner, repo string, number int, reviewID int64) error
|
||||
}
|
||||
|
||||
type Identity interface {
|
||||
GetAuthenticatedUser(ctx context.Context) (string, error)
|
||||
}
|
||||
|
||||
// Client combines all for callers that need everything
|
||||
type Client interface {
|
||||
PRReader
|
||||
FileReader
|
||||
Reviewer
|
||||
Identity
|
||||
}
|
||||
```
|
||||
|
||||
### Types
|
||||
|
||||
Use what gitea/ already has. Move to vcs/types.go or re-export.
|
||||
|
||||
```go
|
||||
type PullRequest struct { ... } // from gitea.PullRequest
|
||||
type ChangedFile struct { ... } // from gitea.ChangedFile
|
||||
type ContentEntry struct { ... } // from gitea.ContentEntry
|
||||
type Review struct { ... } // from gitea.Review
|
||||
type ReviewRequest struct { ... } // new, for PostReview input
|
||||
type ReviewComment struct { ... } // from gitea.ReviewComment
|
||||
```
|
||||
|
||||
### Adapter responsibilities
|
||||
|
||||
Each adapter (gitea, github) handles:
|
||||
- API URL construction
|
||||
- Auth header format (`token` vs `Bearer`)
|
||||
- Request/response mapping
|
||||
- Comment schema translation (line numbers, commit IDs, etc.)
|
||||
|
||||
---
|
||||
|
||||
## Part 4: Test Plan
|
||||
|
||||
### Unit Tests (mock HTTP)
|
||||
|
||||
```
|
||||
github/
|
||||
pr_test.go # TestGetPullRequest, TestGetDiff, TestGetFiles
|
||||
files_test.go # TestGetFileContent, TestListContents
|
||||
review_test.go # TestPostReview, TestListReviews, TestDeleteReview
|
||||
identity_test.go # TestGetAuthenticatedUser
|
||||
```
|
||||
|
||||
Per method: happy path, 404, 401, 429, malformed response.
|
||||
|
||||
### Integration Tests
|
||||
|
||||
Against github.com/aweiker/ai-core-review-bot:
|
||||
- Fetch real PR
|
||||
- Fetch real file
|
||||
- Post + delete review (clean up)
|
||||
|
||||
### End-to-End
|
||||
|
||||
Open PR on test repo, run full review-bot, verify review appears.
|
||||
|
||||
---
|
||||
|
||||
## Part 5: Implementation Phases
|
||||
|
||||
### Phase 1: Extract interfaces from gitea/
|
||||
|
||||
**Work:**
|
||||
- Create `vcs/interfaces.go` with interfaces extracted from gitea/client.go signatures
|
||||
- Create `vcs/types.go` — move or alias types from gitea/
|
||||
- Verify gitea.Client satisfies vcs.Client (compile-time check)
|
||||
|
||||
**Exit criteria:** `var _ vcs.Client = (*gitea.Client)(nil)` compiles.
|
||||
|
||||
---
|
||||
|
||||
### Phase 2: Gitea adapter (if needed)
|
||||
|
||||
**Work:**
|
||||
- If gitea.Client method signatures don't match exactly, create wrapper
|
||||
- Keep gitea/ working exactly as before
|
||||
|
||||
**Exit criteria:** Existing tests pass. No behavior change.
|
||||
|
||||
---
|
||||
|
||||
### Phase 3: GitHub client — PRReader
|
||||
|
||||
**Work:**
|
||||
- `github/client.go` — struct, constructor, HTTP helpers
|
||||
- `github/pr.go` — GetPullRequest, GetPullRequestDiff, GetPullRequestFiles
|
||||
- Unit tests
|
||||
|
||||
**Exit criteria:** `go test ./github/...` passes for PR methods.
|
||||
|
||||
---
|
||||
|
||||
### Phase 4: GitHub client — FileReader
|
||||
|
||||
**Work:**
|
||||
- `github/files.go` — GetFileContent, ListContents
|
||||
- Unit tests
|
||||
|
||||
**Exit criteria:** Unit tests pass.
|
||||
|
||||
---
|
||||
|
||||
### Phase 5: GitHub client — Reviewer + Identity
|
||||
|
||||
**Work:**
|
||||
- `github/review.go` — PostReview, ListReviews, DeleteReview
|
||||
- `github/identity.go` — GetAuthenticatedUser
|
||||
- Unit tests
|
||||
|
||||
**Exit criteria:** Unit tests pass.
|
||||
|
||||
---
|
||||
|
||||
### Phase 6: Integration tests
|
||||
|
||||
**Work:**
|
||||
- `integration/github_test.go`
|
||||
- Test against real GitHub
|
||||
|
||||
**Exit criteria:** All integration tests pass.
|
||||
|
||||
---
|
||||
|
||||
### Phase 7: Wire into cmd/review-bot
|
||||
|
||||
**Work:**
|
||||
- Add `--provider github|gitea` flag (default: gitea for backward compat)
|
||||
- Select client based on flag
|
||||
- Update to use vcs interfaces where it makes sense
|
||||
|
||||
**Exit criteria:**
|
||||
- `./review-bot --provider github ...` works
|
||||
- `./review-bot --provider gitea ...` works (same as before)
|
||||
- Existing Gitea workflows unchanged
|
||||
|
||||
---
|
||||
|
||||
### Phase 8: GitHub Actions workflow + releases
|
||||
|
||||
**Work:**
|
||||
- `.github/workflows/ci.yml` — test on PR
|
||||
- `.github/workflows/release.yml` — publish binary to GitHub releases
|
||||
- `.github/actions/review/action.yml` — composite action
|
||||
- Action downloads binary from github.com/aweiker/ai-core-review-bot releases
|
||||
|
||||
**Exit criteria:**
|
||||
- CI runs on github.com/aweiker/ai-core-review-bot
|
||||
- Release creates downloadable binary
|
||||
- Review action posts review successfully
|
||||
|
||||
---
|
||||
|
||||
## Part 6: Decisions
|
||||
|
||||
| Question | Decision |
|
||||
|----------|----------|
|
||||
| Auth token | Workflow `GITHUB_TOKEN` (automatic) |
|
||||
| Binary distribution | GitHub releases on aweiker/ai-core-review-bot |
|
||||
| Comment schema | Adapter's job — translate ReviewComment to platform format |
|
||||
| Default provider | `gitea` for backward compatibility |
|
||||
| Shared types | vcs/types.go (extracted from gitea/) |
|
||||
| Platform-specific features | Stay on concrete client, not interface |
|
||||
|
||||
---
|
||||
|
||||
## Summary
|
||||
|
||||
8 phases. Start by extracting interfaces from working gitea/ code, not inventing them. GitHub implements the same interfaces. Each phase has clear exit criteria.
|
||||
+215
-17
@@ -11,9 +11,11 @@ import (
|
||||
"fmt"
|
||||
"io"
|
||||
"log/slog"
|
||||
"net"
|
||||
"net/http"
|
||||
"net/url"
|
||||
"strings"
|
||||
"syscall"
|
||||
"time"
|
||||
)
|
||||
|
||||
@@ -39,12 +41,26 @@ func IsNotFound(err error) bool {
|
||||
return errors.As(err, &apiErr) && apiErr.StatusCode == http.StatusNotFound
|
||||
}
|
||||
|
||||
// IsServerError reports whether an error is an API 5xx response.
|
||||
func IsServerError(err error) bool {
|
||||
var apiErr *APIError
|
||||
return errors.As(err, &apiErr) && apiErr.StatusCode >= 500 && apiErr.StatusCode < 600
|
||||
}
|
||||
|
||||
// Client interacts with the Gitea API.
|
||||
// A Client is safe for concurrent use by multiple goroutines.
|
||||
type Client struct {
|
||||
baseURL string
|
||||
token string
|
||||
http *http.Client
|
||||
|
||||
// RetryBackoff defines the delays between retry attempts.
|
||||
// RetryBackoff[i] is the delay before attempt i+1 (after attempt i fails).
|
||||
// If nil, defaults to {1s, 2s}. Set to shorter durations in tests.
|
||||
//
|
||||
// This field must be configured before the first request is made.
|
||||
// Modifying it while requests are in flight is not safe.
|
||||
RetryBackoff []time.Duration
|
||||
}
|
||||
|
||||
// NewClient creates a new Gitea API client.
|
||||
@@ -56,6 +72,12 @@ func NewClient(baseURL, token string) *Client {
|
||||
}
|
||||
}
|
||||
|
||||
// SetHTTPClient sets the underlying HTTP client used for requests.
|
||||
// This is intended for testing to inject mock transports.
|
||||
func (c *Client) SetHTTPClient(hc *http.Client) {
|
||||
c.http = hc
|
||||
}
|
||||
|
||||
// PullRequest holds relevant PR metadata.
|
||||
type PullRequest struct {
|
||||
Title string `json:"title"`
|
||||
@@ -210,24 +232,185 @@ func (c *Client) PostReview(ctx context.Context, owner, repo string, number int,
|
||||
return &review, nil
|
||||
}
|
||||
|
||||
// isTemporaryNetError reports whether err is a temporary network error worth retrying.
|
||||
// This includes connection refused, network unreachable, connection reset, and DNS
|
||||
// timeouts. It explicitly excludes permanent errors like permission denied or
|
||||
// "no such host" DNS failures.
|
||||
func isTemporaryNetError(err error) bool {
|
||||
if err == nil {
|
||||
return false
|
||||
}
|
||||
|
||||
// Check for OpError and inspect the underlying syscall error.
|
||||
// Not all OpErrors are transient — permission denied, for example, is permanent.
|
||||
var opErr *net.OpError
|
||||
if errors.As(err, &opErr) {
|
||||
return isRetriableSyscallError(opErr.Err)
|
||||
}
|
||||
|
||||
// DNS errors: only retry on timeout, not on "no such host" which is permanent.
|
||||
var dnsErr *net.DNSError
|
||||
if errors.As(err, &dnsErr) {
|
||||
return dnsErr.IsTimeout
|
||||
}
|
||||
|
||||
// Check for net.Error with Timeout() (Temporary is deprecated)
|
||||
var netErr net.Error
|
||||
if errors.As(err, &netErr) {
|
||||
return netErr.Timeout()
|
||||
}
|
||||
|
||||
return false
|
||||
}
|
||||
|
||||
// isRetriableSyscallError reports whether the underlying error from a net.OpError
|
||||
// is a transient syscall error worth retrying.
|
||||
func isRetriableSyscallError(err error) bool {
|
||||
if err == nil {
|
||||
return false
|
||||
}
|
||||
|
||||
// Check for syscall.Errno directly or wrapped
|
||||
var errno syscall.Errno
|
||||
if errors.As(err, &errno) {
|
||||
switch errno {
|
||||
case syscall.ECONNREFUSED, // connection refused — server not listening
|
||||
syscall.ECONNRESET, // connection reset by peer
|
||||
syscall.ENETUNREACH, // network unreachable
|
||||
syscall.EHOSTUNREACH, // host unreachable
|
||||
syscall.ETIMEDOUT: // connection timed out
|
||||
return true
|
||||
default:
|
||||
// EACCES, EPERM, etc. are permanent — don't retry
|
||||
return false
|
||||
}
|
||||
}
|
||||
|
||||
// If we can't identify the specific syscall error, be conservative and retry.
|
||||
// This handles wrapped errors or platform-specific error types.
|
||||
// The retry count is limited, so erring on the side of retrying is safe.
|
||||
return true
|
||||
}
|
||||
|
||||
// redactURL strips query parameters from a URL for safe logging.
|
||||
// This prevents accidental exposure of sensitive data that future callers
|
||||
// might pass via query strings.
|
||||
func redactURL(rawURL string) string {
|
||||
parsed, err := url.Parse(rawURL)
|
||||
if err != nil {
|
||||
// If we cannot parse it, return a safe placeholder rather than
|
||||
// potentially logging something sensitive.
|
||||
return "[invalid URL]"
|
||||
}
|
||||
if parsed.RawQuery != "" {
|
||||
parsed.RawQuery = "[redacted]"
|
||||
}
|
||||
return parsed.String()
|
||||
}
|
||||
|
||||
// sanitizeErrorForLog returns a loggable version of an error that omits
|
||||
// potentially sensitive content like response bodies. For APIError, only
|
||||
// the status code is included; for other errors, the type is preserved.
|
||||
func sanitizeErrorForLog(err error) string {
|
||||
if err == nil {
|
||||
return "<nil>"
|
||||
}
|
||||
var apiErr *APIError
|
||||
if errors.As(err, &apiErr) {
|
||||
return fmt.Sprintf("HTTP %d", apiErr.StatusCode)
|
||||
}
|
||||
return err.Error()
|
||||
}
|
||||
|
||||
// doGet performs an HTTP GET request with retry on 5xx errors and temporary
|
||||
// network errors. Retries up to 3 times with exponential backoff (1s, 2s delays
|
||||
// by default; configurable via Client.RetryBackoff for testing).
|
||||
func (c *Client) doGet(ctx context.Context, reqURL string) ([]byte, error) {
|
||||
req, err := http.NewRequestWithContext(ctx, http.MethodGet, reqURL, nil)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
const maxAttempts = 3
|
||||
// backoff[i] is the delay before attempt i+1 (i.e., after attempt i fails).
|
||||
// First attempt (i=0) has no delay; retries wait 1s then 2s by default.
|
||||
backoff := c.RetryBackoff
|
||||
if backoff == nil {
|
||||
backoff = []time.Duration{1 * time.Second, 2 * time.Second}
|
||||
}
|
||||
req.Header.Set("Authorization", "token "+c.token)
|
||||
|
||||
resp, err := c.http.Do(req)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
defer resp.Body.Close()
|
||||
// maxErrorBodyBytes limits how much of an error response body we read
|
||||
// to protect against malicious servers sending unbounded data.
|
||||
const maxErrorBodyBytes = 64 * 1024 // 64 KB
|
||||
|
||||
if resp.StatusCode < 200 || resp.StatusCode >= 300 {
|
||||
body, _ := io.ReadAll(resp.Body)
|
||||
return nil, &APIError{StatusCode: resp.StatusCode, Body: string(body)}
|
||||
var lastErr error
|
||||
for attempt := 0; attempt < maxAttempts; attempt++ {
|
||||
if attempt > 0 {
|
||||
// Determine delay: use backoff slice if available, otherwise retry immediately.
|
||||
// An empty RetryBackoff slice means "retry without delay" — this is intentional
|
||||
// as the caller explicitly configured no delays.
|
||||
var delay time.Duration
|
||||
if attempt-1 < len(backoff) {
|
||||
delay = backoff[attempt-1]
|
||||
}
|
||||
|
||||
if delay > 0 {
|
||||
slog.Warn("retrying request after error",
|
||||
"attempt", attempt+1,
|
||||
"url", redactURL(reqURL),
|
||||
"delay", delay.String(),
|
||||
"lastError", sanitizeErrorForLog(lastErr))
|
||||
|
||||
timer := time.NewTimer(delay)
|
||||
select {
|
||||
case <-timer.C:
|
||||
case <-ctx.Done():
|
||||
timer.Stop()
|
||||
return nil, ctx.Err()
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
req, err := http.NewRequestWithContext(ctx, http.MethodGet, reqURL, nil)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
req.Header.Set("Authorization", "token "+c.token)
|
||||
|
||||
resp, err := c.http.Do(req)
|
||||
if err != nil {
|
||||
// Always capture the error for consistent return at loop end.
|
||||
// This ensures both network errors and HTTP 5xx return lastErr.
|
||||
lastErr = err
|
||||
|
||||
// Only retry temporary network errors when attempts remain.
|
||||
if attempt < maxAttempts-1 && isTemporaryNetError(err) {
|
||||
slog.Warn("temporary network error, will retry",
|
||||
"attempt", attempt+1,
|
||||
"url", redactURL(reqURL),
|
||||
"error", err)
|
||||
continue
|
||||
}
|
||||
// Non-retryable network error or final attempt exhausted.
|
||||
return nil, lastErr
|
||||
}
|
||||
if resp.StatusCode >= 200 && resp.StatusCode < 300 {
|
||||
body, err := io.ReadAll(resp.Body)
|
||||
resp.Body.Close()
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
return body, nil
|
||||
}
|
||||
|
||||
// Error path: limit how much we read from potentially malicious server
|
||||
errBody, _ := io.ReadAll(io.LimitReader(resp.Body, maxErrorBodyBytes))
|
||||
resp.Body.Close()
|
||||
|
||||
lastErr = &APIError{StatusCode: resp.StatusCode, Body: string(errBody)}
|
||||
|
||||
// Only retry on 5xx server errors
|
||||
if resp.StatusCode < 500 || resp.StatusCode >= 600 {
|
||||
return nil, lastErr
|
||||
}
|
||||
}
|
||||
return io.ReadAll(resp.Body)
|
||||
|
||||
return nil, lastErr
|
||||
}
|
||||
|
||||
// escapePath escapes each segment of a relative file path for use in URLs.
|
||||
@@ -251,7 +434,13 @@ type ContentEntry struct {
|
||||
|
||||
// ListContents lists files and directories at a given path in a repo.
|
||||
// Pass an empty path to list the repository root.
|
||||
// If the path points to a file (not a directory), Gitea returns a single
|
||||
// object instead of an array; this method normalizes both cases to a slice.
|
||||
func (c *Client) ListContents(ctx context.Context, owner, repo, path string) ([]ContentEntry, error) {
|
||||
// Normalize "." to empty string — Gitea API rejects "." with 500
|
||||
if path == "." {
|
||||
path = ""
|
||||
}
|
||||
var reqURL string
|
||||
if path == "" {
|
||||
reqURL = fmt.Sprintf("%s/api/v1/repos/%s/%s/contents", c.baseURL, url.PathEscape(owner), url.PathEscape(repo))
|
||||
@@ -264,7 +453,16 @@ func (c *Client) ListContents(ctx context.Context, owner, repo, path string) ([]
|
||||
}
|
||||
var entries []ContentEntry
|
||||
if err := json.Unmarshal(body, &entries); err != nil {
|
||||
return nil, fmt.Errorf("parse contents JSON: %w", err)
|
||||
// Gitea returns a single object (not an array) when path is a file
|
||||
var single ContentEntry
|
||||
if err2 := json.Unmarshal(body, &single); err2 != nil {
|
||||
return nil, fmt.Errorf("parse contents JSON: %w", err)
|
||||
}
|
||||
// Guard against empty/malformed responses
|
||||
if single.Name == "" && single.Path == "" {
|
||||
return nil, fmt.Errorf("parse contents JSON: empty response for path %q", path)
|
||||
}
|
||||
entries = []ContentEntry{single}
|
||||
}
|
||||
return entries, nil
|
||||
}
|
||||
@@ -317,9 +515,9 @@ func (c *Client) GetAllFilesInPath(ctx context.Context, owner, repo, path string
|
||||
|
||||
// Review represents a pull request review from the Gitea API.
|
||||
type Review struct {
|
||||
ID int64 `json:"id"`
|
||||
Body string `json:"body"`
|
||||
User struct {
|
||||
ID int64 `json:"id"`
|
||||
Body string `json:"body"`
|
||||
User struct {
|
||||
Login string `json:"login"`
|
||||
} `json:"user"`
|
||||
State string `json:"state"`
|
||||
|
||||
+406
-5
@@ -6,10 +6,14 @@ import (
|
||||
"errors"
|
||||
"fmt"
|
||||
"io"
|
||||
"net"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"strings"
|
||||
"sync/atomic"
|
||||
"syscall"
|
||||
"testing"
|
||||
"time"
|
||||
)
|
||||
|
||||
func TestGetPullRequest(t *testing.T) {
|
||||
@@ -276,11 +280,64 @@ func TestListContents(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestListContents_DotPath(t *testing.T) {
|
||||
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
// "." should be normalized to empty path, which hits the root contents endpoint
|
||||
if r.URL.Path != "/api/v1/repos/owner/repo/contents" {
|
||||
t.Errorf("expected root contents path, got: %s", r.URL.Path)
|
||||
}
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
fmt.Fprintf(w, `[{"name":"README.md","path":"README.md","type":"file"}]`)
|
||||
}))
|
||||
defer server.Close()
|
||||
|
||||
client := NewClient(server.URL, "test-token")
|
||||
entries, err := client.ListContents(context.Background(), "owner", "repo", ".")
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
if len(entries) != 1 {
|
||||
t.Fatalf("expected 1 entry, got %d", len(entries))
|
||||
}
|
||||
if entries[0].Name != "README.md" {
|
||||
t.Errorf("expected README.md, got %s", entries[0].Name)
|
||||
}
|
||||
}
|
||||
|
||||
func TestListContents_FilePath(t *testing.T) {
|
||||
// Gitea returns a single object (not an array) when path is a file
|
||||
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
if r.URL.Path != "/api/v1/repos/owner/repo/contents/README.md" {
|
||||
t.Errorf("unexpected path: %s", r.URL.Path)
|
||||
}
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
// Single object, not an array
|
||||
fmt.Fprintf(w, `{"name":"README.md","path":"README.md","type":"file"}`)
|
||||
}))
|
||||
defer server.Close()
|
||||
|
||||
client := NewClient(server.URL, "test-token")
|
||||
entries, err := client.ListContents(context.Background(), "owner", "repo", "README.md")
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
if len(entries) != 1 {
|
||||
t.Fatalf("expected 1 entry, got %d", len(entries))
|
||||
}
|
||||
if entries[0].Name != "README.md" {
|
||||
t.Errorf("expected README.md, got %s", entries[0].Name)
|
||||
}
|
||||
if entries[0].Type != "file" {
|
||||
t.Errorf("expected type file, got %s", entries[0].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)
|
||||
// Gitea returns a single object (not array) when path is a file
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
fmt.Fprintf(w, `{"name":"README.md","path":"README.md","type":"file"}`)
|
||||
return
|
||||
}
|
||||
if r.URL.Path == "/api/v1/repos/owner/repo/raw/README.md" {
|
||||
@@ -584,9 +641,9 @@ func TestGetAllFilesInPath_403Propagates(t *testing.T) {
|
||||
|
||||
func TestIsNotFound(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
err error
|
||||
want bool
|
||||
name string
|
||||
err error
|
||||
want bool
|
||||
}{
|
||||
{"nil error", nil, false},
|
||||
{"non-API error", fmt.Errorf("network timeout"), false},
|
||||
@@ -743,3 +800,347 @@ func TestResolveComment_Error(t *testing.T) {
|
||||
t.Fatal("expected error for 404 response")
|
||||
}
|
||||
}
|
||||
|
||||
func TestIsServerError(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"}, false},
|
||||
{"500 APIError", &APIError{StatusCode: 500, Body: "server error"}, true},
|
||||
{"502 APIError", &APIError{StatusCode: 502, Body: "bad gateway"}, true},
|
||||
{"503 APIError", &APIError{StatusCode: 503, Body: "unavailable"}, true},
|
||||
{"599 APIError", &APIError{StatusCode: 599, Body: "edge case"}, true},
|
||||
{"600 not server error", &APIError{StatusCode: 600, Body: "edge"}, false},
|
||||
{"400 not server error", &APIError{StatusCode: 400, Body: "bad request"}, false},
|
||||
{"wrapped 500", fmt.Errorf("fetch: %w", &APIError{StatusCode: 500, Body: "err"}), true},
|
||||
{"wrapped 404", fmt.Errorf("fetch: %w", &APIError{StatusCode: 404, Body: "err"}), false},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
got := IsServerError(tt.err)
|
||||
if got != tt.want {
|
||||
t.Errorf("IsServerError(%v) = %v, want %v", tt.err, got, tt.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestDoGet_RetriesOn500(t *testing.T) {
|
||||
attempts := 0
|
||||
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
attempts++
|
||||
if attempts < 3 {
|
||||
w.WriteHeader(http.StatusInternalServerError)
|
||||
w.Write([]byte(`{"message":"transient error"}`))
|
||||
return
|
||||
}
|
||||
w.WriteHeader(http.StatusOK)
|
||||
w.Write([]byte(`{"data":"success"}`))
|
||||
}))
|
||||
defer server.Close()
|
||||
|
||||
client := NewClient(server.URL, "test-token")
|
||||
// Use short backoff for fast tests
|
||||
client.RetryBackoff = []time.Duration{1 * time.Millisecond, 1 * time.Millisecond}
|
||||
|
||||
body, err := client.doGet(context.Background(), server.URL+"/test")
|
||||
if err != nil {
|
||||
t.Fatalf("expected success after retry, got error: %v", err)
|
||||
}
|
||||
if string(body) != `{"data":"success"}` {
|
||||
t.Errorf("body = %q, want %q", string(body), `{"data":"success"}`)
|
||||
}
|
||||
if attempts != 3 {
|
||||
t.Errorf("attempts = %d, want 3", attempts)
|
||||
}
|
||||
}
|
||||
|
||||
func TestDoGet_FailsAfterMaxRetries(t *testing.T) {
|
||||
attempts := 0
|
||||
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
attempts++
|
||||
w.WriteHeader(http.StatusInternalServerError)
|
||||
w.Write([]byte(`{"message":"persistent error"}`))
|
||||
}))
|
||||
defer server.Close()
|
||||
|
||||
client := NewClient(server.URL, "test-token")
|
||||
// Use short backoff for fast tests
|
||||
client.RetryBackoff = []time.Duration{1 * time.Millisecond, 1 * time.Millisecond}
|
||||
|
||||
_, err := client.doGet(context.Background(), server.URL+"/test")
|
||||
if err == nil {
|
||||
t.Fatal("expected error after max retries")
|
||||
}
|
||||
var apiErr *APIError
|
||||
if !errors.As(err, &apiErr) {
|
||||
t.Fatalf("expected APIError, got: %v", err)
|
||||
}
|
||||
if apiErr.StatusCode != http.StatusInternalServerError {
|
||||
t.Errorf("status = %d, want 500", apiErr.StatusCode)
|
||||
}
|
||||
if attempts != 3 {
|
||||
t.Errorf("attempts = %d, want 3 (max retries)", attempts)
|
||||
}
|
||||
}
|
||||
|
||||
func TestDoGet_NoRetryOn4xx(t *testing.T) {
|
||||
attempts := 0
|
||||
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
attempts++
|
||||
w.WriteHeader(http.StatusForbidden)
|
||||
w.Write([]byte(`{"message":"forbidden"}`))
|
||||
}))
|
||||
defer server.Close()
|
||||
|
||||
client := NewClient(server.URL, "test-token")
|
||||
_, err := client.doGet(context.Background(), server.URL+"/test")
|
||||
if err == nil {
|
||||
t.Fatal("expected error for 403")
|
||||
}
|
||||
var apiErr *APIError
|
||||
if !errors.As(err, &apiErr) {
|
||||
t.Fatalf("expected APIError, got: %v", err)
|
||||
}
|
||||
if apiErr.StatusCode != http.StatusForbidden {
|
||||
t.Errorf("status = %d, want 403", apiErr.StatusCode)
|
||||
}
|
||||
if attempts != 1 {
|
||||
t.Errorf("attempts = %d, want 1 (no retry on 4xx)", attempts)
|
||||
}
|
||||
}
|
||||
|
||||
func TestDoGet_RespectsContextCancellation(t *testing.T) {
|
||||
attempts := 0
|
||||
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
attempts++
|
||||
w.WriteHeader(http.StatusInternalServerError)
|
||||
w.Write([]byte(`{"message":"error"}`))
|
||||
}))
|
||||
defer server.Close()
|
||||
|
||||
ctx, cancel := context.WithCancel(context.Background())
|
||||
|
||||
client := NewClient(server.URL, "test-token")
|
||||
// Use longer backoff to give us time to cancel during the wait
|
||||
client.RetryBackoff = []time.Duration{100 * time.Millisecond, 100 * time.Millisecond}
|
||||
|
||||
// Cancel after first attempt returns and retry begins
|
||||
go func() {
|
||||
time.Sleep(20 * time.Millisecond)
|
||||
cancel()
|
||||
}()
|
||||
|
||||
_, err := client.doGet(ctx, server.URL+"/test")
|
||||
if err == nil {
|
||||
t.Fatal("expected error on context cancellation")
|
||||
}
|
||||
// Should have made 1 attempt, then context cancelled during backoff
|
||||
if attempts != 1 {
|
||||
t.Errorf("attempts = %d, expected 1 before context cancel during backoff", attempts)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
// mockTransport is a test helper that returns errors for the first N calls,
|
||||
// then delegates to a real server.
|
||||
type mockTransport struct {
|
||||
failCount int32 // number of failures remaining (atomic)
|
||||
failErr error // error to return on failure
|
||||
realServer *httptest.Server
|
||||
attemptsMade atomic.Int32 // tracks total attempts
|
||||
}
|
||||
|
||||
func (m *mockTransport) RoundTrip(req *http.Request) (*http.Response, error) {
|
||||
m.attemptsMade.Add(1)
|
||||
remaining := atomic.AddInt32(&m.failCount, -1)
|
||||
if remaining >= 0 {
|
||||
// Still have failures to return
|
||||
return nil, m.failErr
|
||||
}
|
||||
// Redirect to real server
|
||||
req.URL.Host = m.realServer.Listener.Addr().String()
|
||||
req.URL.Scheme = "http"
|
||||
return http.DefaultTransport.RoundTrip(req)
|
||||
}
|
||||
|
||||
func TestDoGet_RetriesOnTemporaryNetError(t *testing.T) {
|
||||
// Real server that will handle successful requests
|
||||
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
w.WriteHeader(http.StatusOK)
|
||||
w.Write([]byte(`{"status":"ok"}`))
|
||||
}))
|
||||
defer server.Close()
|
||||
|
||||
// Mock transport: fail twice with ECONNREFUSED, then succeed
|
||||
mt := &mockTransport{
|
||||
failCount: 2,
|
||||
failErr: &net.OpError{Op: "dial", Net: "tcp", Err: syscall.ECONNREFUSED},
|
||||
realServer: server,
|
||||
}
|
||||
|
||||
client := NewClient("http://fake-host/", "test-token")
|
||||
client.SetHTTPClient(&http.Client{Transport: mt})
|
||||
client.RetryBackoff = []time.Duration{1 * time.Millisecond, 1 * time.Millisecond}
|
||||
|
||||
body, err := client.doGet(context.Background(), "http://fake-host/test")
|
||||
if err != nil {
|
||||
t.Fatalf("expected success after retries, got error: %v", err)
|
||||
}
|
||||
if string(body) != `{"status":"ok"}` {
|
||||
t.Errorf("body = %q, want %q", string(body), `{"status":"ok"}`)
|
||||
}
|
||||
|
||||
// Should have made exactly 3 attempts: 2 failures + 1 success
|
||||
if got := mt.attemptsMade.Load(); got != 3 {
|
||||
t.Errorf("attempts = %d, want 3 (2 failures + 1 success)", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestIsTemporaryNetError(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
err error
|
||||
want bool
|
||||
}{
|
||||
{"nil error", nil, false},
|
||||
{"plain error", fmt.Errorf("some error"), false},
|
||||
// OpError with retriable syscall errors
|
||||
{"OpError ECONNREFUSED", &net.OpError{Op: "dial", Err: syscall.ECONNREFUSED}, true},
|
||||
{"OpError ECONNRESET", &net.OpError{Op: "read", Err: syscall.ECONNRESET}, true},
|
||||
{"OpError ENETUNREACH", &net.OpError{Op: "dial", Err: syscall.ENETUNREACH}, true},
|
||||
{"OpError EHOSTUNREACH", &net.OpError{Op: "dial", Err: syscall.EHOSTUNREACH}, true},
|
||||
{"OpError ETIMEDOUT", &net.OpError{Op: "dial", Err: syscall.ETIMEDOUT}, true},
|
||||
// OpError with permanent syscall errors — should NOT retry
|
||||
{"OpError EACCES", &net.OpError{Op: "dial", Err: syscall.EACCES}, false},
|
||||
{"OpError EPERM", &net.OpError{Op: "dial", Err: syscall.EPERM}, false},
|
||||
// OpError with unknown inner error — conservative retry
|
||||
{"OpError unknown inner", &net.OpError{Op: "dial", Err: fmt.Errorf("unknown")}, true},
|
||||
// DNS errors
|
||||
{"DNS timeout", &net.DNSError{IsTimeout: true}, true},
|
||||
{"DNS no such host", &net.DNSError{IsTimeout: false, Name: "bad.host"}, false},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
got := isTemporaryNetError(tt.err)
|
||||
if got != tt.want {
|
||||
t.Errorf("isTemporaryNetError(%v) = %v, want %v", tt.err, got, tt.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestIsRetriableSyscallError(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
err error
|
||||
want bool
|
||||
}{
|
||||
{"nil", nil, false},
|
||||
{"ECONNREFUSED", syscall.ECONNREFUSED, true},
|
||||
{"ECONNRESET", syscall.ECONNRESET, true},
|
||||
{"ENETUNREACH", syscall.ENETUNREACH, true},
|
||||
{"EHOSTUNREACH", syscall.EHOSTUNREACH, true},
|
||||
{"ETIMEDOUT", syscall.ETIMEDOUT, true},
|
||||
{"EACCES (permanent)", syscall.EACCES, false},
|
||||
{"EPERM (permanent)", syscall.EPERM, false},
|
||||
{"ENOENT (permanent)", syscall.ENOENT, false},
|
||||
{"unknown error", fmt.Errorf("something"), true}, // conservative retry
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
got := isRetriableSyscallError(tt.err)
|
||||
if got != tt.want {
|
||||
t.Errorf("isRetriableSyscallError(%v) = %v, want %v", tt.err, got, tt.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestRedactURL(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
input string
|
||||
want string
|
||||
}{
|
||||
{
|
||||
name: "no query params",
|
||||
input: "https://gitea.example.com/api/v1/repos/owner/repo/pulls/1",
|
||||
want: "https://gitea.example.com/api/v1/repos/owner/repo/pulls/1",
|
||||
},
|
||||
{
|
||||
name: "with query params - redacts",
|
||||
input: "https://gitea.example.com/api/v1/repos/owner/repo/raw/file?ref=main",
|
||||
want: "https://gitea.example.com/api/v1/repos/owner/repo/raw/file?[redacted]",
|
||||
},
|
||||
{
|
||||
name: "multiple query params",
|
||||
input: "https://example.com/path?token=secret&page=1",
|
||||
want: "https://example.com/path?[redacted]",
|
||||
},
|
||||
{
|
||||
name: "invalid URL",
|
||||
input: "://invalid",
|
||||
want: "[invalid URL]",
|
||||
},
|
||||
{
|
||||
name: "empty string",
|
||||
input: "",
|
||||
want: "",
|
||||
},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
got := redactURL(tt.input)
|
||||
if got != tt.want {
|
||||
t.Errorf("redactURL(%q) = %q, want %q", tt.input, got, tt.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestSanitizeErrorForLog(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
err error
|
||||
want string
|
||||
}{
|
||||
{
|
||||
name: "nil error",
|
||||
err: nil,
|
||||
want: "<nil>",
|
||||
},
|
||||
{
|
||||
name: "APIError omits body",
|
||||
err: &APIError{StatusCode: 500, Body: "internal error: database connection failed"},
|
||||
want: "HTTP 500",
|
||||
},
|
||||
{
|
||||
name: "APIError with large body still only shows status",
|
||||
err: &APIError{StatusCode: 502, Body: strings.Repeat("x", 1000)},
|
||||
want: "HTTP 502",
|
||||
},
|
||||
{
|
||||
name: "non-API error preserved",
|
||||
err: fmt.Errorf("connection refused"),
|
||||
want: "connection refused",
|
||||
},
|
||||
{
|
||||
name: "wrapped APIError",
|
||||
err: fmt.Errorf("request failed: %w", &APIError{StatusCode: 503, Body: "service unavailable"}),
|
||||
want: "HTTP 503",
|
||||
},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
got := sanitizeErrorForLog(tt.err)
|
||||
if got != tt.want {
|
||||
t.Errorf("sanitizeErrorForLog() = %q, want %q", got, tt.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,27 @@
|
||||
//go:build phase2
|
||||
|
||||
package vcs_test
|
||||
|
||||
import (
|
||||
"gitea.weiker.me/rodin/review-bot/gitea"
|
||||
"gitea.weiker.me/rodin/review-bot/vcs"
|
||||
)
|
||||
|
||||
// Compile-time assertion: documents the gap between gitea.Client and vcs.Client.
|
||||
// Guarded by the "phase2" build tag — enable once the Gitea adapter bridges these gaps:
|
||||
//
|
||||
// 1. PostReview signature mismatch:
|
||||
// gitea.Client: PostReview(ctx, owner, repo, number, event, body string, comments []gitea.ReviewComment)
|
||||
// vcs.Reviewer: PostReview(ctx, owner, repo, number, req vcs.ReviewRequest)
|
||||
//
|
||||
// 2. GetFileContent signature mismatch:
|
||||
// gitea.Client: GetFileContent(ctx, owner, repo, filepath string) [no ref; uses default branch]
|
||||
// vcs.FileReader: GetFileContent(ctx, owner, repo, path, ref string)
|
||||
// (gitea.Client has GetFileContentRef for the ref variant)
|
||||
//
|
||||
// 3. ReviewComment type mismatch:
|
||||
// gitea.ReviewComment uses NewPosition int64 (Gitea line-number convention)
|
||||
// vcs.ReviewComment uses Position int (GitHub diff-position convention)
|
||||
//
|
||||
// The Gitea adapter (Phase 2) will wrap gitea.Client to bridge these gaps.
|
||||
var _ vcs.Client = (*gitea.Client)(nil)
|
||||
@@ -0,0 +1,40 @@
|
||||
// Package vcs defines the shared VCS client interface and supporting types.
|
||||
// Platform adapters (gitea, github) implement these interfaces so the core
|
||||
// review logic can work with any VCS platform without platform-specific code.
|
||||
package vcs
|
||||
|
||||
import "context"
|
||||
|
||||
// PRReader can fetch pull request metadata, diffs, and changed files.
|
||||
type PRReader interface {
|
||||
GetPullRequest(ctx context.Context, owner, repo string, number int) (*PullRequest, error)
|
||||
GetPullRequestDiff(ctx context.Context, owner, repo string, number int) (string, error)
|
||||
GetPullRequestFiles(ctx context.Context, owner, repo string, number int) ([]ChangedFile, error)
|
||||
}
|
||||
|
||||
// FileReader can fetch file contents and list directory entries.
|
||||
type FileReader interface {
|
||||
GetFileContent(ctx context.Context, owner, repo, path, ref string) (string, error)
|
||||
ListContents(ctx context.Context, owner, repo, path string) ([]ContentEntry, error)
|
||||
}
|
||||
|
||||
// Reviewer can post, list, and delete pull request reviews.
|
||||
type Reviewer interface {
|
||||
PostReview(ctx context.Context, owner, repo string, number int, req ReviewRequest) (*Review, error)
|
||||
ListReviews(ctx context.Context, owner, repo string, number int) ([]Review, error)
|
||||
DeleteReview(ctx context.Context, owner, repo string, number int, reviewID int64) error
|
||||
}
|
||||
|
||||
// Identity can report who the authenticated user is.
|
||||
type Identity interface {
|
||||
GetAuthenticatedUser(ctx context.Context) (string, error)
|
||||
}
|
||||
|
||||
// Client is the full VCS interface: PR reads, file reads, review management, and identity.
|
||||
// Platform adapters (gitea, github) implement this interface.
|
||||
type Client interface {
|
||||
PRReader
|
||||
FileReader
|
||||
Reviewer
|
||||
Identity
|
||||
}
|
||||
@@ -0,0 +1,82 @@
|
||||
package vcs
|
||||
|
||||
// ReviewEvent is the event type for a pull request review action.
|
||||
// Adapters must translate these action constants to/from platform-native values.
|
||||
// For example, Gitea uses "APPROVED" as both action and state, while GitHub
|
||||
// uses "APPROVE" for the action and returns "approved" as the state.
|
||||
type ReviewEvent string
|
||||
|
||||
const (
|
||||
// ReviewEventApprove approves the pull request.
|
||||
ReviewEventApprove ReviewEvent = "APPROVE"
|
||||
// ReviewEventRequestChanges requests changes to the pull request.
|
||||
ReviewEventRequestChanges ReviewEvent = "REQUEST_CHANGES"
|
||||
// ReviewEventComment posts a review comment without approval or rejection.
|
||||
ReviewEventComment ReviewEvent = "COMMENT"
|
||||
)
|
||||
|
||||
// HeadRef identifies the source branch and latest commit of a pull request.
|
||||
type HeadRef struct {
|
||||
SHA string `json:"sha"`
|
||||
Ref string `json:"ref"`
|
||||
}
|
||||
|
||||
// UserInfo identifies a user by login name.
|
||||
type UserInfo struct {
|
||||
Login string `json:"login"`
|
||||
}
|
||||
|
||||
// PullRequest holds relevant PR metadata.
|
||||
type PullRequest struct {
|
||||
Title string `json:"title"`
|
||||
Body string `json:"body"`
|
||||
Head HeadRef `json:"head"`
|
||||
}
|
||||
|
||||
// ChangedFile represents a file modified in a PR.
|
||||
type ChangedFile struct {
|
||||
Filename string `json:"filename"`
|
||||
Status string `json:"status"`
|
||||
}
|
||||
|
||||
// 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"
|
||||
}
|
||||
|
||||
// Review represents a pull request review.
|
||||
type Review struct {
|
||||
ID int64 `json:"id"`
|
||||
Body string `json:"body"`
|
||||
User UserInfo `json:"user"`
|
||||
State string `json:"state"`
|
||||
Stale bool `json:"stale"`
|
||||
CommitID string `json:"commit_id"`
|
||||
}
|
||||
|
||||
// ReviewComment represents an inline comment in a review.
|
||||
// All adapters use GitHub diff-position convention:
|
||||
// - Position is a 1-indexed offset from the @@ hunk line in the unified diff.
|
||||
// - CommitID identifies the commit the comment is anchored to.
|
||||
// It is optional; omit (empty string) for review-level comments that are
|
||||
// not attached to a specific commit.
|
||||
//
|
||||
// Adapters are responsible for translating to/from platform-native formats
|
||||
// (e.g. Gitea uses line numbers; GitHub uses diff positions natively).
|
||||
type ReviewComment struct {
|
||||
Path string `json:"path"`
|
||||
Position int `json:"position"` // diff-position: 1-indexed offset from @@ hunk line
|
||||
CommitID string `json:"commit_id"`
|
||||
Body string `json:"body"`
|
||||
}
|
||||
|
||||
// ReviewRequest is the payload for posting a review.
|
||||
type ReviewRequest struct {
|
||||
// Body is the top-level review comment.
|
||||
Body string `json:"body"`
|
||||
// Event is the review action (approve, request changes, or comment).
|
||||
Event ReviewEvent `json:"event"`
|
||||
Comments []ReviewComment `json:"comments,omitempty"`
|
||||
}
|
||||
+142
@@ -0,0 +1,142 @@
|
||||
package vcs
|
||||
|
||||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
"strconv"
|
||||
"strings"
|
||||
)
|
||||
|
||||
// GetAllFilesInPath recursively fetches all file contents under a path using the
|
||||
// provided FileReader. Returns a map of filepath -> content for all files found.
|
||||
// If the path points to an empty directory, returns an empty map.
|
||||
func GetAllFilesInPath(ctx context.Context, client FileReader, owner, repo, path string) (map[string]string, error) {
|
||||
results := make(map[string]string)
|
||||
|
||||
entries, err := client.ListContents(ctx, owner, repo, path)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("list contents %q: %w", path, err)
|
||||
}
|
||||
|
||||
for _, entry := range entries {
|
||||
switch entry.Type {
|
||||
case "file":
|
||||
content, err := client.GetFileContent(ctx, owner, repo, entry.Path, "")
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("get file %q: %w", entry.Path, err)
|
||||
}
|
||||
results[entry.Path] = content
|
||||
case "dir":
|
||||
subResults, err := GetAllFilesInPath(ctx, client, owner, repo, entry.Path)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("recurse into %q: %w", entry.Path, err)
|
||||
}
|
||||
for k, v := range subResults {
|
||||
results[k] = v
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return results, nil
|
||||
}
|
||||
|
||||
// BuildLineToPositionMap parses a unified diff and returns a per-file map of
|
||||
// new-file line number → diff-position.
|
||||
//
|
||||
// Position is a 1-indexed offset from the @@ hunk line (GitHub convention):
|
||||
// - The @@ hunk header itself counts as position 1 (for the first hunk)
|
||||
// - Every subsequent content line (context, addition, deletion) increments position
|
||||
// - A second @@ hunk in the same file continues the count; it does not reset
|
||||
// - Addition and context lines have new-file line numbers and are mapped
|
||||
// - Deletion lines have a position but no new-file line; they are not in the map
|
||||
// - "\ No newline at end of file" markers do not count as a position
|
||||
//
|
||||
// Returns: map[filename]map[newFileLine]diffPosition
|
||||
func BuildLineToPositionMap(diff string) map[string]map[int]int {
|
||||
result := make(map[string]map[int]int)
|
||||
|
||||
lines := strings.Split(diff, "\n")
|
||||
var currentFile string
|
||||
var position int
|
||||
var newLine int
|
||||
|
||||
for _, line := range lines {
|
||||
// Detect new file in diff — resets position counter per file.
|
||||
if strings.HasPrefix(line, "+++ b/") {
|
||||
currentFile = strings.TrimPrefix(line, "+++ b/")
|
||||
position = 0
|
||||
newLine = 0
|
||||
if result[currentFile] == nil {
|
||||
result[currentFile] = make(map[int]int)
|
||||
}
|
||||
continue
|
||||
}
|
||||
|
||||
// Skip deleted-file target (no new lines to map).
|
||||
if strings.HasPrefix(line, "+++ /dev/null") {
|
||||
currentFile = ""
|
||||
continue
|
||||
}
|
||||
|
||||
// Skip metadata lines that are not part of position counting.
|
||||
if strings.HasPrefix(line, "diff --git") ||
|
||||
strings.HasPrefix(line, "--- ") ||
|
||||
strings.HasPrefix(line, "index ") ||
|
||||
strings.HasPrefix(line, "\\") {
|
||||
continue
|
||||
}
|
||||
|
||||
// Parse hunk headers — the @@ line itself occupies a position.
|
||||
if strings.HasPrefix(line, "@@") && currentFile != "" {
|
||||
position++
|
||||
newLine = parseHunkNewStart(line)
|
||||
continue
|
||||
}
|
||||
|
||||
if currentFile == "" {
|
||||
continue
|
||||
}
|
||||
|
||||
// Content lines within a hunk.
|
||||
switch {
|
||||
case strings.HasPrefix(line, "+"):
|
||||
// Addition: maps to both a position and a new-file line.
|
||||
position++
|
||||
result[currentFile][newLine] = position
|
||||
newLine++
|
||||
case strings.HasPrefix(line, "-"):
|
||||
// Deletion: occupies a position but has no new-file line number.
|
||||
position++
|
||||
case strings.HasPrefix(line, " "):
|
||||
// Context line: maps to both a position and a new-file line.
|
||||
position++
|
||||
result[currentFile][newLine] = position
|
||||
newLine++
|
||||
}
|
||||
// Any other line (empty trailing lines from Split, etc.) is ignored.
|
||||
}
|
||||
|
||||
return result
|
||||
}
|
||||
|
||||
// parseHunkNewStart extracts the new-file starting line number from a hunk header.
|
||||
// Format: @@ -old_start[,old_count] +new_start[,new_count] @@[ optional section heading]
|
||||
func parseHunkNewStart(hunkLine string) int {
|
||||
// Find the +N part after the first @@
|
||||
plusIdx := strings.Index(hunkLine, "+")
|
||||
if plusIdx < 0 {
|
||||
return 1
|
||||
}
|
||||
rest := hunkLine[plusIdx+1:]
|
||||
|
||||
// Read digits until comma, space, or end.
|
||||
if idx := strings.IndexAny(rest, ", @"); idx > 0 {
|
||||
rest = rest[:idx]
|
||||
}
|
||||
|
||||
n, err := strconv.Atoi(rest)
|
||||
if err != nil {
|
||||
return 1
|
||||
}
|
||||
return n
|
||||
}
|
||||
@@ -0,0 +1,456 @@
|
||||
package vcs_test
|
||||
|
||||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
"testing"
|
||||
|
||||
"gitea.weiker.me/rodin/review-bot/vcs"
|
||||
)
|
||||
|
||||
// mockFileReader implements vcs.FileReader for testing.
|
||||
type mockFileReader struct {
|
||||
contents map[string]string // path -> content
|
||||
dirs map[string][]vcs.ContentEntry // path -> entries
|
||||
}
|
||||
|
||||
func (m *mockFileReader) GetFileContent(_ context.Context, _, _, path, _ string) (string, error) {
|
||||
content, ok := m.contents[path]
|
||||
if !ok {
|
||||
return "", fmt.Errorf("file not found: %s", path)
|
||||
}
|
||||
return content, nil
|
||||
}
|
||||
|
||||
func (m *mockFileReader) ListContents(_ context.Context, _, _, path string) ([]vcs.ContentEntry, error) {
|
||||
entries, ok := m.dirs[path]
|
||||
if !ok {
|
||||
return nil, fmt.Errorf("directory not found: %s", path)
|
||||
}
|
||||
return entries, nil
|
||||
}
|
||||
|
||||
// --- GetAllFilesInPath tests ---
|
||||
|
||||
func TestGetAllFilesInPath_EmptyDir(t *testing.T) {
|
||||
mock := &mockFileReader{
|
||||
dirs: map[string][]vcs.ContentEntry{
|
||||
"src": {},
|
||||
},
|
||||
}
|
||||
|
||||
result, err := vcs.GetAllFilesInPath(context.Background(), mock, "owner", "repo", "src")
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
if len(result) != 0 {
|
||||
t.Errorf("expected empty map, got %d entries", len(result))
|
||||
}
|
||||
}
|
||||
|
||||
func TestGetAllFilesInPath_FlatDir(t *testing.T) {
|
||||
mock := &mockFileReader{
|
||||
dirs: map[string][]vcs.ContentEntry{
|
||||
"src": {
|
||||
{Name: "main.go", Path: "src/main.go", Type: "file"},
|
||||
{Name: "util.go", Path: "src/util.go", Type: "file"},
|
||||
},
|
||||
},
|
||||
contents: map[string]string{
|
||||
"src/main.go": "package main",
|
||||
"src/util.go": "package main\n\nfunc helper() {}",
|
||||
},
|
||||
}
|
||||
|
||||
result, err := vcs.GetAllFilesInPath(context.Background(), mock, "owner", "repo", "src")
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
if len(result) != 2 {
|
||||
t.Fatalf("expected 2 files, got %d", len(result))
|
||||
}
|
||||
if result["src/main.go"] != "package main" {
|
||||
t.Errorf("unexpected content for main.go: %q", result["src/main.go"])
|
||||
}
|
||||
if result["src/util.go"] != "package main\n\nfunc helper() {}" {
|
||||
t.Errorf("unexpected content for util.go: %q", result["src/util.go"])
|
||||
}
|
||||
}
|
||||
|
||||
func TestGetAllFilesInPath_NestedDirs(t *testing.T) {
|
||||
mock := &mockFileReader{
|
||||
dirs: map[string][]vcs.ContentEntry{
|
||||
"src": {
|
||||
{Name: "main.go", Path: "src/main.go", Type: "file"},
|
||||
{Name: "pkg", Path: "src/pkg", Type: "dir"},
|
||||
},
|
||||
"src/pkg": {
|
||||
{Name: "lib.go", Path: "src/pkg/lib.go", Type: "file"},
|
||||
{Name: "internal", Path: "src/pkg/internal", Type: "dir"},
|
||||
},
|
||||
"src/pkg/internal": {
|
||||
{Name: "helper.go", Path: "src/pkg/internal/helper.go", Type: "file"},
|
||||
},
|
||||
},
|
||||
contents: map[string]string{
|
||||
"src/main.go": "package main",
|
||||
"src/pkg/lib.go": "package pkg",
|
||||
"src/pkg/internal/helper.go": "package internal",
|
||||
},
|
||||
}
|
||||
|
||||
result, err := vcs.GetAllFilesInPath(context.Background(), mock, "owner", "repo", "src")
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
if len(result) != 3 {
|
||||
t.Fatalf("expected 3 files, got %d", len(result))
|
||||
}
|
||||
if result["src/main.go"] != "package main" {
|
||||
t.Errorf("unexpected content for main.go")
|
||||
}
|
||||
if result["src/pkg/lib.go"] != "package pkg" {
|
||||
t.Errorf("unexpected content for lib.go")
|
||||
}
|
||||
if result["src/pkg/internal/helper.go"] != "package internal" {
|
||||
t.Errorf("unexpected content for helper.go")
|
||||
}
|
||||
}
|
||||
|
||||
func TestGetAllFilesInPath_Mixed(t *testing.T) {
|
||||
mock := &mockFileReader{
|
||||
dirs: map[string][]vcs.ContentEntry{
|
||||
"root": {
|
||||
{Name: "README.md", Path: "root/README.md", Type: "file"},
|
||||
{Name: "empty", Path: "root/empty", Type: "dir"},
|
||||
{Name: "docs", Path: "root/docs", Type: "dir"},
|
||||
},
|
||||
"root/empty": {},
|
||||
"root/docs": {
|
||||
{Name: "guide.md", Path: "root/docs/guide.md", Type: "file"},
|
||||
},
|
||||
},
|
||||
contents: map[string]string{
|
||||
"root/README.md": "# Hello",
|
||||
"root/docs/guide.md": "## Guide",
|
||||
},
|
||||
}
|
||||
|
||||
result, err := vcs.GetAllFilesInPath(context.Background(), mock, "owner", "repo", "root")
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
if len(result) != 2 {
|
||||
t.Fatalf("expected 2 files, got %d", len(result))
|
||||
}
|
||||
if result["root/README.md"] != "# Hello" {
|
||||
t.Errorf("unexpected content for README.md")
|
||||
}
|
||||
if result["root/docs/guide.md"] != "## Guide" {
|
||||
t.Errorf("unexpected content for guide.md")
|
||||
}
|
||||
}
|
||||
|
||||
func TestGetAllFilesInPath_ListContentsError(t *testing.T) {
|
||||
mock := &mockFileReader{
|
||||
dirs: map[string][]vcs.ContentEntry{},
|
||||
}
|
||||
|
||||
_, err := vcs.GetAllFilesInPath(context.Background(), mock, "owner", "repo", "missing")
|
||||
if err == nil {
|
||||
t.Fatal("expected error for missing directory")
|
||||
}
|
||||
}
|
||||
|
||||
func TestGetAllFilesInPath_GetFileContentError(t *testing.T) {
|
||||
mock := &mockFileReader{
|
||||
dirs: map[string][]vcs.ContentEntry{
|
||||
"src": {
|
||||
{Name: "bad.go", Path: "src/bad.go", Type: "file"},
|
||||
},
|
||||
},
|
||||
contents: map[string]string{},
|
||||
}
|
||||
|
||||
_, err := vcs.GetAllFilesInPath(context.Background(), mock, "owner", "repo", "src")
|
||||
if err == nil {
|
||||
t.Fatal("expected error when file content fetch fails")
|
||||
}
|
||||
}
|
||||
|
||||
// --- BuildLineToPositionMap tests ---
|
||||
|
||||
func TestBuildLineToPositionMap_SingleHunk(t *testing.T) {
|
||||
diff := `diff --git a/main.go b/main.go
|
||||
index abc..def 100644
|
||||
--- a/main.go
|
||||
+++ b/main.go
|
||||
@@ -5,7 +5,8 @@ package main
|
||||
import "fmt"
|
||||
|
||||
func main() {
|
||||
- fmt.Println("old")
|
||||
+ fmt.Println("new")
|
||||
+ fmt.Println("added")
|
||||
return
|
||||
}
|
||||
`
|
||||
|
||||
result := vcs.BuildLineToPositionMap(diff)
|
||||
|
||||
fileMap, ok := result["main.go"]
|
||||
if !ok {
|
||||
t.Fatal("expected main.go in result")
|
||||
}
|
||||
|
||||
// @@ line is position 1
|
||||
// " import \"fmt\"" -> pos 2, newLine 5
|
||||
// " " -> pos 3, newLine 6
|
||||
// " func main() {" -> pos 4, newLine 7
|
||||
// "-\tfmt.Println..." -> pos 5, no new line
|
||||
// "+\tfmt.Println..." -> pos 6, newLine 8
|
||||
// "+\tfmt.Println..." -> pos 7, newLine 9
|
||||
// " \treturn" -> pos 8, newLine 10
|
||||
// " }" -> pos 9, newLine 11
|
||||
|
||||
expected := map[int]int{
|
||||
5: 2,
|
||||
6: 3,
|
||||
7: 4,
|
||||
8: 6,
|
||||
9: 7,
|
||||
10: 8,
|
||||
11: 9,
|
||||
}
|
||||
|
||||
for line, wantPos := range expected {
|
||||
gotPos, exists := fileMap[line]
|
||||
if !exists {
|
||||
t.Errorf("line %d: expected position %d, but line not in map", line, wantPos)
|
||||
continue
|
||||
}
|
||||
if gotPos != wantPos {
|
||||
t.Errorf("line %d: expected position %d, got %d", line, wantPos, gotPos)
|
||||
}
|
||||
}
|
||||
|
||||
if len(fileMap) != len(expected) {
|
||||
t.Errorf("expected %d entries, got %d", len(expected), len(fileMap))
|
||||
}
|
||||
}
|
||||
|
||||
func TestBuildLineToPositionMap_MultiHunk(t *testing.T) {
|
||||
diff := `diff --git a/main.go b/main.go
|
||||
--- a/main.go
|
||||
+++ b/main.go
|
||||
@@ -1,3 +1,4 @@
|
||||
package main
|
||||
|
||||
+import "fmt"
|
||||
func main() {
|
||||
@@ -10,3 +11,4 @@ func main() {
|
||||
return
|
||||
+ // extra
|
||||
}
|
||||
`
|
||||
|
||||
result := vcs.BuildLineToPositionMap(diff)
|
||||
|
||||
fileMap, ok := result["main.go"]
|
||||
if !ok {
|
||||
t.Fatal("expected main.go in result")
|
||||
}
|
||||
|
||||
// First hunk:
|
||||
// @@ line -> pos 1
|
||||
// " package main" -> pos 2, newLine 1
|
||||
// " " -> pos 3, newLine 2
|
||||
// "+import..." -> pos 4, newLine 3
|
||||
// " func main.." -> pos 5, newLine 4
|
||||
//
|
||||
// Second hunk (position continues!):
|
||||
// @@ line -> pos 6
|
||||
// " \treturn" -> pos 7, newLine 11
|
||||
// "+\t// extra" -> pos 8, newLine 12
|
||||
// " }" -> pos 9, newLine 13
|
||||
|
||||
expected := map[int]int{
|
||||
1: 2,
|
||||
2: 3,
|
||||
3: 4,
|
||||
4: 5,
|
||||
11: 7,
|
||||
12: 8,
|
||||
13: 9,
|
||||
}
|
||||
|
||||
for line, wantPos := range expected {
|
||||
gotPos, exists := fileMap[line]
|
||||
if !exists {
|
||||
t.Errorf("line %d: expected position %d, but line not in map", line, wantPos)
|
||||
continue
|
||||
}
|
||||
if gotPos != wantPos {
|
||||
t.Errorf("line %d: expected position %d, got %d", line, wantPos, gotPos)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestBuildLineToPositionMap_DeletionLinesNotInMap(t *testing.T) {
|
||||
diff := `diff --git a/old.go b/old.go
|
||||
--- a/old.go
|
||||
+++ b/old.go
|
||||
@@ -1,3 +1,1 @@
|
||||
-line one
|
||||
-line two
|
||||
remaining
|
||||
`
|
||||
|
||||
result := vcs.BuildLineToPositionMap(diff)
|
||||
|
||||
fileMap, ok := result["old.go"]
|
||||
if !ok {
|
||||
t.Fatal("expected old.go in result")
|
||||
}
|
||||
|
||||
// @@ line -> pos 1
|
||||
// "-line one" -> pos 2, no new line
|
||||
// "-line two" -> pos 3, no new line
|
||||
// " remaining" -> pos 4, newLine 1
|
||||
|
||||
if pos, ok := fileMap[1]; !ok || pos != 4 {
|
||||
t.Errorf("line 1: expected position 4, got %d (exists=%v)", pos, ok)
|
||||
}
|
||||
if len(fileMap) != 1 {
|
||||
t.Errorf("expected 1 entry (only 'remaining'), got %d", len(fileMap))
|
||||
}
|
||||
}
|
||||
|
||||
func TestBuildLineToPositionMap_MultipleFiles(t *testing.T) {
|
||||
diff := `diff --git a/a.go b/a.go
|
||||
--- a/a.go
|
||||
+++ b/a.go
|
||||
@@ -1,2 +1,3 @@
|
||||
package a
|
||||
+// added
|
||||
|
||||
diff --git a/b.go b/b.go
|
||||
new file mode 100644
|
||||
--- /dev/null
|
||||
+++ b/b.go
|
||||
@@ -0,0 +1,3 @@
|
||||
+package b
|
||||
+
|
||||
+func B() {}
|
||||
`
|
||||
|
||||
result := vcs.BuildLineToPositionMap(diff)
|
||||
|
||||
// File a.go
|
||||
aMap, ok := result["a.go"]
|
||||
if !ok {
|
||||
t.Fatal("expected a.go in result")
|
||||
}
|
||||
// @@ line -> pos 1
|
||||
// " package a" -> pos 2, newLine 1
|
||||
// "+// added" -> pos 3, newLine 2
|
||||
// " " -> pos 4, newLine 3
|
||||
if aMap[1] != 2 {
|
||||
t.Errorf("a.go line 1: expected pos 2, got %d", aMap[1])
|
||||
}
|
||||
if aMap[2] != 3 {
|
||||
t.Errorf("a.go line 2: expected pos 3, got %d", aMap[2])
|
||||
}
|
||||
if aMap[3] != 4 {
|
||||
t.Errorf("a.go line 3: expected pos 4, got %d", aMap[3])
|
||||
}
|
||||
|
||||
// File b.go — position resets for new file
|
||||
bMap, ok := result["b.go"]
|
||||
if !ok {
|
||||
t.Fatal("expected b.go in result")
|
||||
}
|
||||
// @@ line -> pos 1
|
||||
// "+package b" -> pos 2, newLine 1
|
||||
// "+" -> pos 3, newLine 2
|
||||
// "+func B() {}" -> pos 4, newLine 3
|
||||
if bMap[1] != 2 {
|
||||
t.Errorf("b.go line 1: expected pos 2, got %d", bMap[1])
|
||||
}
|
||||
if bMap[2] != 3 {
|
||||
t.Errorf("b.go line 2: expected pos 3, got %d", bMap[2])
|
||||
}
|
||||
if bMap[3] != 4 {
|
||||
t.Errorf("b.go line 3: expected pos 4, got %d", bMap[3])
|
||||
}
|
||||
}
|
||||
|
||||
func TestBuildLineToPositionMap_EmptyDiff(t *testing.T) {
|
||||
result := vcs.BuildLineToPositionMap("")
|
||||
if len(result) != 0 {
|
||||
t.Errorf("expected empty map for empty diff, got %d entries", len(result))
|
||||
}
|
||||
}
|
||||
|
||||
func TestBuildLineToPositionMap_NoNewlineMarker(t *testing.T) {
|
||||
diff := `diff --git a/a.go b/a.go
|
||||
--- a/a.go
|
||||
+++ b/a.go
|
||||
@@ -1,2 +1,2 @@
|
||||
-old
|
||||
+new
|
||||
\ No newline at end of file
|
||||
`
|
||||
|
||||
result := vcs.BuildLineToPositionMap(diff)
|
||||
|
||||
fileMap := result["a.go"]
|
||||
// @@ line -> pos 1
|
||||
// "-old" -> pos 2
|
||||
// "+new" -> pos 3, newLine 1
|
||||
// "\ No.." -> not counted
|
||||
if fileMap[1] != 3 {
|
||||
t.Errorf("line 1: expected position 3, got %d", fileMap[1])
|
||||
}
|
||||
if len(fileMap) != 1 {
|
||||
t.Errorf("expected 1 entry, got %d", len(fileMap))
|
||||
}
|
||||
}
|
||||
|
||||
func TestBuildLineToPositionMap_SingleLineHunk(t *testing.T) {
|
||||
diff := `diff --git a/single.go b/single.go
|
||||
--- a/single.go
|
||||
+++ b/single.go
|
||||
@@ -1 +1 @@
|
||||
-old
|
||||
+new
|
||||
`
|
||||
|
||||
result := vcs.BuildLineToPositionMap(diff)
|
||||
|
||||
fileMap := result["single.go"]
|
||||
// @@ line -> pos 1
|
||||
// "-old" -> pos 2
|
||||
// "+new" -> pos 3, newLine 1
|
||||
if fileMap[1] != 3 {
|
||||
t.Errorf("line 1: expected position 3, got %d", fileMap[1])
|
||||
}
|
||||
}
|
||||
|
||||
func TestBuildLineToPositionMap_DeletedFile(t *testing.T) {
|
||||
diff := `diff --git a/deleted.go b/deleted.go
|
||||
deleted file mode 100644
|
||||
--- a/deleted.go
|
||||
+++ /dev/null
|
||||
@@ -1,3 +0,0 @@
|
||||
-package deleted
|
||||
-
|
||||
-func Gone() {}
|
||||
`
|
||||
|
||||
result := vcs.BuildLineToPositionMap(diff)
|
||||
|
||||
if _, ok := result["deleted.go"]; ok {
|
||||
t.Error("deleted file should not appear in result")
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user