Compare commits

...

6 Commits

Author SHA1 Message Date
Rodin aade891129 docs: add package-level documentation
CI / test (pull_request) Successful in 14s
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Failing after 55s
CI / review (gpt-5-mini, gpt, GPT_REVIEW_TOKEN) (pull_request) Failing after 1m6s
Per go-patterns/package-design.md, every package needs a doc comment.
Added to gitea, llm, and review packages.
2026-05-01 14:54:58 -07:00
Rodin 7b42de67ca fix: handle empty path in ListContents (root listing)
CI / test (pull_request) Successful in 14s
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 1m4s
CI / review (gpt-5-mini, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 1m8s
Empty path now yields /contents instead of /contents/ (trailing slash).
Added doc comment noting empty path = repo root.
2026-05-01 14:46:40 -07:00
Rodin dd2661fe14 fix: address all review findings from PR #17
CI / test (pull_request) Successful in 13s
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 52s
CI / review (gpt-5-mini, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 1m12s
- Rename all remaining url locals to reqURL (consistency)
- Use http.MethodGet/http.MethodPost constants
- Document escapePath: relative paths only, double-encoding expected
- Add TestEscapePath with 7 edge cases (empty, spaces, #, deep, encoded)
2026-05-01 14:40:19 -07:00
Rodin 98a4772f30 fix: path-escape file paths and eliminate url package shadowing
CI / test (pull_request) Successful in 14s
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 55s
CI / review (gpt-5-mini, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 1m48s
- Add escapePath() helper: escapes each path segment individually
  (preserves slashes as separators, escapes spaces/#/? etc)
- Apply to GetFileContent, GetFileContentRef, ListContents
- Rename doGet parameter from url to reqURL (avoids shadowing net/url)
- Rename local variables in GetFileContent/ListContents for consistency

Addresses remaining findings from PR #16 review.
2026-05-01 14:33:18 -07:00
rodin fc23b6ebe9 Merge pull request 'fix: quick wins (#7, #9, #13)' (#16) from fix/quick-wins into main
CI / test (push) Successful in 14s
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (push) Has been skipped
CI / review (gpt-5-mini, gpt, GPT_REVIEW_TOKEN) (push) Has been skipped
2026-05-01 21:30:57 +00:00
Rodin b02ade4f23 fix: quick wins (#7, #9, #13)
CI / test (pull_request) Successful in 13s
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 59s
CI / review (gpt-5-mini, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 1m48s
- Add --version flag and log version on startup (closes #9)
- URL-escape ref query parameter in GetFileContentRef (closes #7)
- Add go vet to release workflow (closes #13)

Renamed local url variable to reqURL to avoid shadowing net/url package.
2026-05-01 14:19:37 -07:00
6 changed files with 78 additions and 19 deletions
+3 -1
View File
@@ -16,7 +16,9 @@ jobs:
go-version: '1.26' go-version: '1.26'
- name: Run tests - name: Run tests
run: go test ./... run: |
go vet ./...
go test ./...
- name: Build binaries - name: Build binaries
run: | run: |
+8
View File
@@ -18,6 +18,7 @@ import (
var version = "dev" var version = "dev"
func main() { func main() {
versionFlag := flag.Bool("version", false, "Print version and exit")
// CLI flags // CLI flags
giteaURL := flag.String("gitea-url", envOrDefault("GITEA_URL", ""), "Gitea instance URL") giteaURL := flag.String("gitea-url", envOrDefault("GITEA_URL", ""), "Gitea instance URL")
repo := flag.String("repo", envOrDefault("GITEA_REPO", ""), "Repository (owner/name)") repo := flag.String("repo", envOrDefault("GITEA_REPO", ""), "Repository (owner/name)")
@@ -36,6 +37,13 @@ func main() {
flag.Parse() flag.Parse()
if *versionFlag {
fmt.Printf("review-bot %s\n", version)
os.Exit(0)
}
log.Printf("review-bot %s", version)
// Validate required fields // Validate required fields
if *giteaURL == "" || *repo == "" || *prNum == "" || *reviewerToken == "" || if *giteaURL == "" || *repo == "" || *prNum == "" || *reviewerToken == "" ||
*llmBaseURL == "" || *llmAPIKey == "" || *llmModel == "" { *llmBaseURL == "" || *llmAPIKey == "" || *llmModel == "" {
+40 -18
View File
@@ -1,3 +1,6 @@
// Package gitea provides a client for the Gitea API.
// It supports pull request operations, file content retrieval,
// and review submission.
package gitea package gitea
import ( import (
@@ -8,6 +11,7 @@ import (
"io" "io"
"log" "log"
"net/http" "net/http"
"net/url"
"strings" "strings"
"time" "time"
) )
@@ -55,8 +59,8 @@ type ChangedFile struct {
// GetPullRequest fetches PR metadata. // GetPullRequest fetches PR metadata.
func (c *Client) GetPullRequest(ctx context.Context, owner, repo string, number int) (*PullRequest, error) { func (c *Client) GetPullRequest(ctx context.Context, owner, repo string, number int) (*PullRequest, error) {
url := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls/%d", c.baseURL, owner, repo, number) reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls/%d", c.baseURL, owner, repo, number)
body, err := c.doGet(ctx, url) body, err := c.doGet(ctx, reqURL)
if err != nil { if err != nil {
return nil, fmt.Errorf("fetch PR: %w", err) return nil, fmt.Errorf("fetch PR: %w", err)
} }
@@ -69,8 +73,8 @@ func (c *Client) GetPullRequest(ctx context.Context, owner, repo string, number
// GetPullRequestDiff fetches the unified diff for a PR. // GetPullRequestDiff fetches the unified diff for a PR.
func (c *Client) GetPullRequestDiff(ctx context.Context, owner, repo string, number int) (string, error) { func (c *Client) GetPullRequestDiff(ctx context.Context, owner, repo string, number int) (string, error) {
url := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls/%d.diff", c.baseURL, owner, repo, number) reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls/%d.diff", c.baseURL, owner, repo, number)
body, err := c.doGet(ctx, url) body, err := c.doGet(ctx, reqURL)
if err != nil { if err != nil {
return "", fmt.Errorf("fetch diff: %w", err) return "", fmt.Errorf("fetch diff: %w", err)
} }
@@ -79,8 +83,8 @@ func (c *Client) GetPullRequestDiff(ctx context.Context, owner, repo string, num
// GetPullRequestFiles fetches the list of files changed in a PR. // GetPullRequestFiles fetches the list of files changed in a PR.
func (c *Client) GetPullRequestFiles(ctx context.Context, owner, repo string, number int) ([]ChangedFile, error) { func (c *Client) GetPullRequestFiles(ctx context.Context, owner, repo string, number int) ([]ChangedFile, error) {
url := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls/%d/files", c.baseURL, owner, repo, number) reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls/%d/files", c.baseURL, owner, repo, number)
body, err := c.doGet(ctx, url) body, err := c.doGet(ctx, reqURL)
if err != nil { if err != nil {
return nil, fmt.Errorf("fetch PR files: %w", err) return nil, fmt.Errorf("fetch PR files: %w", err)
} }
@@ -93,8 +97,8 @@ func (c *Client) GetPullRequestFiles(ctx context.Context, owner, repo string, nu
// GetCommitStatuses fetches CI statuses for a commit SHA. // GetCommitStatuses fetches CI statuses for a commit SHA.
func (c *Client) GetCommitStatuses(ctx context.Context, owner, repo, sha string) ([]CommitStatus, error) { func (c *Client) GetCommitStatuses(ctx context.Context, owner, repo, sha string) ([]CommitStatus, error) {
url := fmt.Sprintf("%s/api/v1/repos/%s/%s/commits/%s/statuses", c.baseURL, owner, repo, sha) reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/commits/%s/statuses", c.baseURL, owner, repo, sha)
body, err := c.doGet(ctx, url) body, err := c.doGet(ctx, reqURL)
if err != nil { if err != nil {
return nil, fmt.Errorf("fetch commit statuses: %w", err) return nil, fmt.Errorf("fetch commit statuses: %w", err)
} }
@@ -107,8 +111,8 @@ func (c *Client) GetCommitStatuses(ctx context.Context, owner, repo, sha string)
// GetFileContent fetches a file from the default branch of a repo. // GetFileContent fetches a file from the default branch of a repo.
func (c *Client) GetFileContent(ctx context.Context, owner, repo, filepath string) (string, error) { func (c *Client) GetFileContent(ctx context.Context, owner, repo, filepath string) (string, error) {
url := fmt.Sprintf("%s/api/v1/repos/%s/%s/raw/%s", c.baseURL, owner, repo, filepath) reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/raw/%s", c.baseURL, owner, repo, escapePath(filepath))
body, err := c.doGet(ctx, url) body, err := c.doGet(ctx, reqURL)
if err != nil { if err != nil {
return "", fmt.Errorf("fetch file %s: %w", filepath, err) return "", fmt.Errorf("fetch file %s: %w", filepath, err)
} }
@@ -117,8 +121,8 @@ func (c *Client) GetFileContent(ctx context.Context, owner, repo, filepath strin
// GetFileContentRef fetches a file from a specific ref (branch/tag/sha) in a repo. // GetFileContentRef fetches a file from a specific ref (branch/tag/sha) in a repo.
func (c *Client) GetFileContentRef(ctx context.Context, owner, repo, filepath, ref string) (string, error) { func (c *Client) GetFileContentRef(ctx context.Context, owner, repo, filepath, ref string) (string, error) {
url := fmt.Sprintf("%s/api/v1/repos/%s/%s/raw/%s?ref=%s", c.baseURL, owner, repo, filepath, ref) reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/raw/%s?ref=%s", c.baseURL, owner, repo, escapePath(filepath), url.QueryEscape(ref))
body, err := c.doGet(ctx, url) body, err := c.doGet(ctx, reqURL)
if err != nil { if err != nil {
return "", fmt.Errorf("fetch file %s@%s: %w", filepath, ref, err) return "", fmt.Errorf("fetch file %s@%s: %w", filepath, ref, err)
} }
@@ -128,7 +132,7 @@ func (c *Client) GetFileContentRef(ctx context.Context, owner, repo, filepath, r
// PostReview submits a review to a PR. // PostReview submits a review to a PR.
// event should be "APPROVED" or "REQUEST_CHANGES". // event should be "APPROVED" or "REQUEST_CHANGES".
func (c *Client) PostReview(ctx context.Context, owner, repo string, number int, event, body string) error { func (c *Client) PostReview(ctx context.Context, owner, repo string, number int, event, body string) error {
url := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls/%d/reviews", c.baseURL, owner, repo, number) reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls/%d/reviews", c.baseURL, owner, repo, number)
payload := struct { payload := struct {
Body string `json:"body"` Body string `json:"body"`
@@ -143,7 +147,7 @@ func (c *Client) PostReview(ctx context.Context, owner, repo string, number int,
return fmt.Errorf("marshal review payload: %w", err) return fmt.Errorf("marshal review payload: %w", err)
} }
req, err := http.NewRequestWithContext(ctx, "POST", url, bytes.NewReader(data)) req, err := http.NewRequestWithContext(ctx, http.MethodPost, reqURL, bytes.NewReader(data))
if err != nil { if err != nil {
return fmt.Errorf("create review request: %w", err) return fmt.Errorf("create review request: %w", err)
} }
@@ -163,8 +167,8 @@ func (c *Client) PostReview(ctx context.Context, owner, repo string, number int,
return nil return nil
} }
func (c *Client) doGet(ctx context.Context, url string) ([]byte, error) { func (c *Client) doGet(ctx context.Context, reqURL string) ([]byte, error) {
req, err := http.NewRequestWithContext(ctx, "GET", url, nil) req, err := http.NewRequestWithContext(ctx, http.MethodGet, reqURL, nil)
if err != nil { if err != nil {
return nil, err return nil, err
} }
@@ -183,6 +187,18 @@ func (c *Client) doGet(ctx context.Context, url string) ([]byte, error) {
return io.ReadAll(resp.Body) return io.ReadAll(resp.Body)
} }
// escapePath escapes each segment of a relative file path for use in URLs.
// Slashes are preserved as path separators; other special characters are escaped.
// Input should be a relative path (no leading slash). Already-encoded segments
// will be double-encoded, which is the desired behavior for user-provided paths.
func escapePath(p string) string {
parts := strings.Split(p, "/")
for i, part := range parts {
parts[i] = url.PathEscape(part)
}
return strings.Join(parts, "/")
}
// ContentEntry represents a file or directory entry from the contents API. // ContentEntry represents a file or directory entry from the contents API.
type ContentEntry struct { type ContentEntry struct {
Name string `json:"name"` Name string `json:"name"`
@@ -191,9 +207,15 @@ type ContentEntry struct {
} }
// ListContents lists files and directories at a given path in a repo. // ListContents lists files and directories at a given path in a repo.
// Pass an empty path to list the repository root.
func (c *Client) ListContents(ctx context.Context, owner, repo, path string) ([]ContentEntry, error) { func (c *Client) ListContents(ctx context.Context, owner, repo, path string) ([]ContentEntry, error) {
url := fmt.Sprintf("%s/api/v1/repos/%s/%s/contents/%s", c.baseURL, owner, repo, path) var reqURL string
body, err := c.doGet(ctx, url) if path == "" {
reqURL = fmt.Sprintf("%s/api/v1/repos/%s/%s/contents", c.baseURL, owner, repo)
} else {
reqURL = fmt.Sprintf("%s/api/v1/repos/%s/%s/contents/%s", c.baseURL, owner, repo, escapePath(path))
}
body, err := c.doGet(ctx, reqURL)
if err != nil { if err != nil {
return nil, fmt.Errorf("list contents %s: %w", path, err) return nil, fmt.Errorf("list contents %s: %w", path, err)
} }
+24
View File
@@ -294,3 +294,27 @@ func TestGetAllFilesInPath_File(t *testing.T) {
t.Errorf("unexpected content: %q", files["README.md"]) t.Errorf("unexpected content: %q", files["README.md"])
} }
} }
func TestEscapePath(t *testing.T) {
tests := []struct {
name string
input string
want string
}{
{"simple", "src/main.go", "src/main.go"},
{"spaces", "my dir/my file.go", "my%20dir/my%20file.go"},
{"special chars", "path/file#1.txt", "path/file%231.txt"},
{"empty", "", ""},
{"single segment", "README.md", "README.md"},
{"nested deep", "a/b/c/d.md", "a/b/c/d.md"},
{"already encoded", "path/file%20name.go", "path/file%2520name.go"},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := escapePath(tt.input)
if got != tt.want {
t.Errorf("escapePath(%q) = %q, want %q", tt.input, got, tt.want)
}
})
}
}
+1
View File
@@ -1,3 +1,4 @@
// Package llm provides a client for OpenAI-compatible chat completion APIs.
package llm package llm
import ( import (
+2
View File
@@ -1,3 +1,5 @@
// Package review builds prompts for AI code review and parses LLM responses
// into structured review results.
package review package review
import ( import (