Merge pull request 'fix: path-escape file paths and eliminate url package shadowing' (#17) from fix/url-escaping-and-shadow into main

gitea/client.go

@@ -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
 
 import (
@@ -56,8 +59,8 @@ type ChangedFile struct {
 
 // GetPullRequest fetches PR metadata.
 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)
-	body, err := c.doGet(ctx, url)
+	reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls/%d", c.baseURL, owner, repo, number)
+	body, err := c.doGet(ctx, reqURL)
 	if err != nil {
 		return nil, fmt.Errorf("fetch PR: %w", err)
 	}
@@ -70,8 +73,8 @@ func (c *Client) GetPullRequest(ctx context.Context, owner, repo string, number
 
 // GetPullRequestDiff fetches the unified diff for a PR.
 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)
-	body, err := c.doGet(ctx, url)
+	reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls/%d.diff", c.baseURL, owner, repo, number)
+	body, err := c.doGet(ctx, reqURL)
 	if err != nil {
 		return "", fmt.Errorf("fetch diff: %w", err)
 	}
@@ -80,8 +83,8 @@ func (c *Client) GetPullRequestDiff(ctx context.Context, owner, repo string, num
 
 // GetPullRequestFiles fetches the list of files changed in a PR.
 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)
-	body, err := c.doGet(ctx, url)
+	reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls/%d/files", c.baseURL, owner, repo, number)
+	body, err := c.doGet(ctx, reqURL)
 	if err != nil {
 		return nil, fmt.Errorf("fetch PR files: %w", err)
 	}
@@ -94,8 +97,8 @@ func (c *Client) GetPullRequestFiles(ctx context.Context, owner, repo string, nu
 
 // GetCommitStatuses fetches CI statuses for a commit SHA.
 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)
-	body, err := c.doGet(ctx, url)
+	reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/commits/%s/statuses", c.baseURL, owner, repo, sha)
+	body, err := c.doGet(ctx, reqURL)
 	if err != nil {
 		return nil, fmt.Errorf("fetch commit statuses: %w", err)
 	}
@@ -108,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.
 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)
-	body, err := c.doGet(ctx, url)
+	reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/raw/%s", c.baseURL, owner, repo, escapePath(filepath))
+	body, err := c.doGet(ctx, reqURL)
 	if err != nil {
 		return "", fmt.Errorf("fetch file %s: %w", filepath, err)
 	}
@@ -118,7 +121,7 @@ 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.
 func (c *Client) GetFileContentRef(ctx context.Context, owner, repo, filepath, ref string) (string, error) {
-	reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/raw/%s?ref=%s", c.baseURL, owner, repo, filepath, url.QueryEscape(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, reqURL)
 	if err != nil {
 		return "", fmt.Errorf("fetch file %s@%s: %w", filepath, ref, err)
@@ -129,7 +132,7 @@ func (c *Client) GetFileContentRef(ctx context.Context, owner, repo, filepath, r
 // PostReview submits a review to a PR.
 // event should be "APPROVED" or "REQUEST_CHANGES".
 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 {
 		Body  string `json:"body"`
@@ -144,7 +147,7 @@ func (c *Client) PostReview(ctx context.Context, owner, repo string, number int,
 		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 {
 		return fmt.Errorf("create review request: %w", err)
 	}
@@ -164,8 +167,8 @@ func (c *Client) PostReview(ctx context.Context, owner, repo string, number int,
 	return nil
 }
 
-func (c *Client) doGet(ctx context.Context, url string) ([]byte, error) {
-	req, err := http.NewRequestWithContext(ctx, "GET", url, nil)
+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
 	}
@@ -184,6 +187,18 @@ func (c *Client) doGet(ctx context.Context, url string) ([]byte, error) {
 	return io.ReadAll(resp.Body)
 }
 
+// escapePath escapes each segment of a relative file path for use in URLs.
+// Slashes are preserved as path separators; other special characters are escaped.
+// Input should be a relative path (no leading slash). Already-encoded segments
+// will be double-encoded, which is the desired behavior for user-provided paths.
+func escapePath(p string) string {
+	parts := strings.Split(p, "/")
+	for i, part := range parts {
+		parts[i] = url.PathEscape(part)
+	}
+	return strings.Join(parts, "/")
+}
+
 // ContentEntry represents a file or directory entry from the contents API.
 type ContentEntry struct {
 	Name string `json:"name"`
@@ -192,9 +207,15 @@ type ContentEntry struct {
 }
 
 // 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) {
-	url := fmt.Sprintf("%s/api/v1/repos/%s/%s/contents/%s", c.baseURL, owner, repo, path)
-	body, err := c.doGet(ctx, url)
+	var reqURL string
+	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 {
 		return nil, fmt.Errorf("list contents %s: %w", path, err)
 	}

gitea/client_test.go

@@ -294,3 +294,27 @@ func TestGetAllFilesInPath_File(t *testing.T) {
 		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)
+			}
+		})
+	}
+}

llm/client.go

@@ -1,3 +1,4 @@
+// Package llm provides a client for OpenAI-compatible chat completion APIs.
 package llm
 
 import (

review/prompt.go

@@ -1,3 +1,5 @@
+// Package review builds prompts for AI code review and parses LLM responses
+// into structured review results.
 package review
 
 import (