Compare commits

..

1 Commits

Author SHA1 Message Date
Rodin 29ab19c94d fix: address PR #75 review findings
PR Ready Gate / clear-labels (pull_request) Successful in 2s
CI / test (pull_request) Successful in 18s
CI / review (anthropic--claude-4.6-sonnet, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 39s
CI / review (gpt-5, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 1m32s
CI / review (gpt-5, security, ., rodin/security-patterns, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 3m18s
MAJOR fixes:
- ci.yml: Add fork protection (github.event.pull_request.head.repo.full_name check)
  to prevent secret exfiltration from malicious fork PRs. Added security comment
  explaining the trust model for this private repo.
- ci.yml: Set GITHUB_SERVER_URL to explicit Gitea URL instead of github.server_url
  since reviews are posted to Gitea, not GitHub.
- release.yml: Set GITEA_URL explicitly to https://gitea.weiker.me since releases
  are created on Gitea.
- action.yml: Change gitea-url default from empty (fallback to github.server_url)
  to explicit https://gitea.weiker.me. Update all internal uses to rely on this
  default rather than falling back to server_url.

MINOR fixes:
- action.yml: Update header comment to reflect dual-platform (Gitea Actions +
  GitHub Actions) support.
- action.yml: Fix repo input description to say it defaults to rodin/review-bot
  for version lookup, matching the actual code behavior.
- pr-ready-gate.yml: Add comments explaining why Gitea URL is hardcoded (intentional:
  we update Gitea PR from GitHub mirror) and noting the PR number matching assumption.

All findings from sonnet-review, gpt-review, and security-review addressed.
2026-05-11 08:52:23 -07:00
8 changed files with 28 additions and 431 deletions
+12 -9
View File
@@ -1,17 +1,17 @@
# This composite action is designed for Gitea Actions runners.
# Gitea Actions supports GitHub Actions syntax including $GITHUB_OUTPUT,
# actions/cache, and actions/checkout.
# Composite action for Gitea Actions and GitHub Actions runners.
# Supports dual-platform deployment: reviews can be triggered from GitHub (mirrored repo)
# or Gitea, but always post results to the Gitea PR.
# 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)'
description: 'Gitea instance URL for API calls and releases (defaults to https://gitea.weiker.me)'
required: false
default: ''
default: 'https://gitea.weiker.me'
repo:
description: 'Repository (owner/name, defaults to current)'
description: 'Repository (owner/name, defaults to rodin/review-bot for version lookup)'
required: false
default: ''
pr-number:
@@ -112,7 +112,8 @@ runs:
id: version
shell: bash
run: |
GITEA_URL="${{ inputs.gitea-url || github.server_url }}"
# Use explicit gitea-url input, falling back to default (https://gitea.weiker.me)
GITEA_URL="${{ inputs.gitea-url }}"
REPO="${{ inputs.repo || 'rodin/review-bot' }}"
if [ "${{ inputs.version }}" = "latest" ]; then
VERSION=$(curl -sSf "${GITEA_URL}/api/v1/repos/${REPO}/releases?limit=1" \
@@ -137,7 +138,8 @@ runs:
if: steps.cache.outputs.cache-hit != 'true'
shell: bash
run: |
GITEA_URL="${{ inputs.gitea-url || github.server_url }}"
# Use explicit gitea-url input for release downloads
GITEA_URL="${{ inputs.gitea-url }}"
REPO="${{ inputs.repo || 'rodin/review-bot' }}"
VERSION="${{ steps.version.outputs.version }}"
BINARY="review-bot-linux-amd64"
@@ -169,7 +171,8 @@ runs:
- name: Run review
shell: bash
env:
GITHUB_SERVER_URL: ${{ inputs.gitea-url || github.server_url }}
# Always use Gitea API - reviews are posted to Gitea regardless of where workflow runs
GITHUB_SERVER_URL: ${{ inputs.gitea-url }}
GITHUB_REPOSITORY: ${{ inputs.repo || github.repository }}
PR_NUMBER: ${{ inputs.pr-number || github.event.pull_request.number }}
REVIEWER_TOKEN: ${{ inputs.reviewer-token }}
+8 -3
View File
@@ -22,9 +22,13 @@ jobs:
# 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
#
# SECURITY: This job runs on pull_request and has access to secrets.
# We restrict to same-repo PRs only (no forks) since this is a private repo
# where PRs only come from trusted actors (rodin/aweiker).
review:
runs-on: ubuntu-24.04
if: github.event_name == 'pull_request'
if: github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name == github.repository
needs: test
strategy:
matrix:
@@ -49,8 +53,9 @@ jobs:
- 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 }}
# Use Gitea API - reviews are posted to Gitea, not GitHub
GITHUB_SERVER_URL: https://gitea.weiker.me
GITHUB_REPOSITORY: rodin/review-bot
PR_NUMBER: ${{ github.event.pull_request.number }}
REVIEWER_TOKEN: ${{ secrets[matrix.token_secret] }}
REVIEWER_NAME: ${{ matrix.name }}
+5
View File
@@ -13,11 +13,16 @@ jobs:
env:
GITEA_TOKEN: ${{ secrets.RODIN_TOKEN }}
run: |
# NOTE: This workflow runs on the GitHub mirror but updates the Gitea PR.
# PR numbers may differ between GitHub and Gitea mirrors in edge cases.
# For this repo, PRs are created on Gitea and mirrored, so numbers match.
PR_NUMBER=${{ github.event.pull_request.number }}
AUTHOR=${{ github.event.pull_request.user.login }}
READY_LABEL_ID=38
SELF_REVIEWED_LABEL_ID=37
# INTENTIONAL: Hardcoded Gitea URL because we always update the Gitea PR,
# not GitHub. The mirror relationship means we want changes on Gitea.
# Remove ready label if present
curl -sS -X DELETE \
-H "Authorization: token $GITEA_TOKEN" \
+3 -2
View File
@@ -37,8 +37,9 @@ jobs:
GITEA_TOKEN: ${{ secrets.RELEASE_TOKEN }}
run: |
VERSION=${GITHUB_REF_NAME}
GITEA_URL="${{ github.server_url }}"
REPO="${{ github.repository }}"
# Releases are created on Gitea, not GitHub - use explicit Gitea URL
GITEA_URL="https://gitea.weiker.me"
REPO="rodin/review-bot"
# Create release (or find existing one for this tag)
HTTP_CODE=$(curl -s -o /tmp/release_response.json -w "%{http_code}" -X POST \
-268
View File
@@ -1,268 +0,0 @@
# 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.
-27
View File
@@ -1,27 +0,0 @@
//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)
-40
View File
@@ -1,40 +0,0 @@
// 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
}
-82
View File
@@ -1,82 +0,0 @@
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"`
}