# VCS Abstraction Layer Design

Issue: #76

## Problem

review-bot currently only works with Gitea. We want it to work on GitHub (github.com and GHE) without maintaining two forks.

## Constraints

- Must be backward compatible — existing Gitea usage unchanged
- Runtime detection — no config flag needed
- Same types — gitea/ and github/ packages already use identical structs
- Minimal diff — don't rewrite, just abstract

## Current State

Two packages exist:
- `gitea/` in rodin/review-bot — Gitea API
- `github/` in strat/review-bot — GitHub API (forked from gitea/, adjusted paths)

Method signatures are identical. Types are identical. Only API endpoints differ.

## Proposed Approach

### Phase 1: Port github/ package

Copy strat/review-bot's `github/` package into rodin/review-bot.

Files:
- `github/client.go`
- `github/client_test.go`  
- `github/diff.go`

No changes to gitea/ package.

### Phase 2: Add vcs/ abstraction

```
vcs/
  interface.go   # Client interface
  client.go      # NewClient() with detection
  doc.go         # package doc
```

**interface.go:**
```go
package vcs

import "context"

// Client abstracts VCS operations. Implemented by gitea.Client and github.Client.
type Client 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)
    GetCommitStatuses(ctx context.Context, owner, repo, sha string) ([]CommitStatus, error)
    GetFileContent(ctx context.Context, owner, repo, filepath string) (string, error)
    GetFileContentRef(ctx context.Context, owner, repo, filepath, ref string) (string, error)
    PostReview(ctx context.Context, owner, repo string, number int, event, body string, comments []ReviewComment) (*Review, error)
    ListContents(ctx context.Context, owner, repo, path string) ([]ContentEntry, error)
    GetAllFilesInPath(ctx context.Context, owner, repo, path string) (map[string]string, error)
    ListReviews(ctx context.Context, owner, repo string, number int) ([]Review, error)
    DeleteReview(ctx context.Context, owner, repo string, number int, reviewID int64) error
    GetAuthenticatedUser(ctx context.Context) (string, error)
}

// Types re-exported from gitea package (canonical source)
type (
    PullRequest   = gitea.PullRequest
    ChangedFile   = gitea.ChangedFile
    CommitStatus  = gitea.CommitStatus
    ReviewComment = gitea.ReviewComment
    ContentEntry  = gitea.ContentEntry
    Review        = gitea.Review
)
```

**client.go:**
```go
package vcs

import (
    "strings"
    
    "gitea.weiker.me/rodin/review-bot/gitea"
    "gitea.weiker.me/rodin/review-bot/github"
)

// NewClient creates a VCS client based on the server URL.
// GitHub detection: github.com, api.github.com, or "github" in domain (GHE).
func NewClient(baseURL, token string) Client {
    if isGitHub(baseURL) {
        return github.NewClient(baseURL, token)
    }
    return gitea.NewClient(baseURL, token)
}

func isGitHub(baseURL string) bool {
    lower := strings.ToLower(baseURL)
    if strings.Contains(lower, "github.com") {
        return true
    }
    // GHE: github.mycompany.com but NOT gitea.mycompany.com
    if strings.Contains(lower, "github") && !strings.Contains(lower, "gitea") {
        return true
    }
    return false
}
```

### Phase 3: Update cmd/review-bot

Change from:
```go
client := gitea.NewClient(baseURL, token)
```

To:
```go
client := vcs.NewClient(baseURL, token)
```

All callers already use the interface implicitly (same method names). This is a one-line change.

## API Differences Handled in github/

| Operation | Gitea | GitHub |
|-----------|-------|--------|
| API prefix | `/api/v1/repos/` | `/repos/` (github.com uses api.github.com) |
| Get diff | `GET /pulls/{n}.diff` | `GET /pulls/{n}` + `Accept: application/vnd.github.diff` |
| Auth header | `Authorization: token X` | `Authorization: Bearer X` |

The github/ package already handles these. No new code needed.

## Methods NOT in interface (Gitea-only)

These methods exist in gitea/ but aren't needed for core review flow:
- `GetTimelineReviewCommentID` — Gitea-specific timeline API
- `GetTimelineReviewCommentIDForReview` — Gitea-specific
- `EditComment` — supersede flow (Gitea-specific)
- `ResolveComment` — Gitea-specific

If a caller needs these, they can type-assert: `if gc, ok := client.(*gitea.Client); ok { ... }`

## Testing Strategy

1. Unit tests for `isGitHub()` detection
2. Existing gitea/ tests unchanged
3. Existing github/ tests from strat fork
4. Integration test: mock server, verify correct client selected

## Phases

| PR | Content | Exit Criteria |
|----|---------|---------------|
| 1 | Port github/ package | `go test ./github/...` passes |
| 2 | Add vcs/ interface | `go build ./...` compiles |
| 3 | Update cmd/review-bot | All existing tests pass, GitHub detection works |

## Open Questions

1. **Type re-export vs duplicate** — Using type aliases (`type PullRequest = gitea.PullRequest`) means callers import vcs and get gitea types. Alternative: duplicate types in vcs/. Preference?

2. **github.com API base URL** — For github.com, API is at `api.github.com`, not `github.com/api`. The github/ package needs to handle this. Does strat's version already?

3. **GHE API path** — GHE uses `/api/v3/repos/`. Verify strat's github/ package handles this.

## Answers to Open Questions

### Q1: Type re-export vs duplicate

**Answer: Type aliases.** 
- Keeps one source of truth (gitea/ package)
- If types ever diverge, we'll know at compile time
- Callers importing `vcs` get clean namespace

### Q2 & Q3: API URL handling

**Answer: NewClient needs URL transformation.**

The github/ package expects the API root:
- github.com → `https://api.github.com`
- GHE → `https://github.concur.com/api/v3`

But callers pass server URLs (from `github.server_url` context):
- github.com → `https://github.com`
- GHE → `https://github.concur.com`

**Updated client.go:**
```go
func NewClient(serverURL, token string) Client {
    if isGitHub(serverURL) {
        apiURL := toGitHubAPIURL(serverURL)
        return github.NewClient(apiURL, token)
    }
    return gitea.NewClient(serverURL, token)
}

func toGitHubAPIURL(serverURL string) string {
    lower := strings.ToLower(serverURL)
    if strings.Contains(lower, "github.com") && !strings.Contains(lower, "api.github.com") {
        return "https://api.github.com"
    }
    // GHE: append /api/v3
    return strings.TrimRight(serverURL, "/") + "/api/v3"
}
```

This keeps the github/ package unchanged (it already handles the API URL format).