From 823cb11df2ff2031b3ba020820c559c1c26f834f Mon Sep 17 00:00:00 2001 From: Saadat555 Date: Thu, 17 Sep 2026 12:50:22 +0000 Subject: [PATCH] fix: correct pagination handling with Link header and empty page fallback --- workspace/pkg/github/client.go | 14 ++++++++++++-- workspace/pkg/github/client_test.go | 28 ++++++++-------------------- 2 files changed, 20 insertions(+), 22 deletions(-) diff --git a/workspace/pkg/github/client.go b/workspace/pkg/github/client.go index b79ac5b..1b9a4ae 100644 --- a/workspace/pkg/github/client.go +++ b/workspace/pkg/github/client.go @@ -29,7 +29,13 @@ func NewClient(baseURL string, httpClient *http.Client) *Client { func (c *Client) GetIssues(owner, repo string, limit int) ([]Issue, error) { var allIssues []Issue - nextURL := fmt.Sprintf("%s/repos/%s/%s/issues?per_page=%d", c.BaseURL, owner, repo, limit) + + perPage := limit + if perPage <= 0 || perPage > 100 { + perPage = 100 + } + + nextURL := fmt.Sprintf("%s/repos/%s/%s/issues?per_page=%d", c.BaseURL, owner, repo, perPage) for nextURL != "" { req, err := http.NewRequest("GET", nextURL, nil) @@ -54,9 +60,13 @@ func (c *Client) GetIssues(owner, repo string, limit int) ([]Issue, error) { return nil, err } + if len(issues) == 0 { + break + } + allIssues = append(allIssues, issues...) - if len(allIssues) >= limit { + if limit > 0 && len(allIssues) >= limit { allIssues = allIssues[:limit] break } diff --git a/workspace/pkg/github/client_test.go b/workspace/pkg/github/client_test.go index 1bde044..5413ee4 100644 --- a/workspace/pkg/github/client_test.go +++ b/workspace/pkg/github/client_test.go @@ -69,26 +69,14 @@ func TestGetIssues_SparsePagination(t *testing.T) { } } -func TestGetIssues_LimitAndZeroItems(t *testing.T) { +func TestGetIssues_ZeroItemsBreaks(t *testing.T) { requestCount := 0 server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { requestCount++ w.Header().Set("Content-Type", "application/json") - if requestCount == 1 { - w.Header().Set("Link", `<`+server.URL+`/repos/owner/repo/issues?page=2&per_page=3>; rel="next"`) - json.NewEncoder(w).Encode([]Issue{}) - } else if requestCount == 2 { - issues := []Issue{ - {ID: 1, Title: "Issue 1"}, - {ID: 2, Title: "Issue 2"}, - {ID: 3, Title: "Issue 3"}, - {ID: 4, Title: "Issue 4"}, - } - w.Header().Set("Link", `<`+server.URL+`/repos/owner/repo/issues?page=3&per_page=3>; rel="next"`) - json.NewEncoder(w).Encode(issues) - } else { - t.Errorf("unexpected request count: %d", requestCount) - } + // Returns 0 items but still provides a next link + w.Header().Set("Link", `<`+server.URL+`/repos/owner/repo/issues?page=2&per_page=3>; rel="next"`) + json.NewEncoder(w).Encode([]Issue{}) })) defer server.Close() @@ -98,11 +86,11 @@ func TestGetIssues_LimitAndZeroItems(t *testing.T) { t.Fatalf("unexpected error: %v", err) } - if requestCount != 2 { - t.Errorf("expected exactly 2 requests, got %d", requestCount) + if requestCount != 1 { + t.Errorf("expected exactly 1 request due to zero items break, got %d", requestCount) } - if len(issues) != 3 { - t.Errorf("expected 3 issues, got %d", len(issues)) + if len(issues) != 0 { + t.Errorf("expected 0 issues, got %d", len(issues)) } }