From e307cdad447cea042471f41e9c6e89a3295a897a Mon Sep 17 00:00:00 2001 From: jearthliu Date: Wed, 12 Aug 2026 10:07:21 +0800 Subject: [PATCH 1/3] fix: stop pagination on empty page even with next link --- workspace/pkg/github/client.go | 26 +++++++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/workspace/pkg/github/client.go b/workspace/pkg/github/client.go index b79ac5b..f41c05f 100644 --- a/workspace/pkg/github/client.go +++ b/workspace/pkg/github/client.go @@ -29,7 +29,15 @@ 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) + + // Per-page size. A non-positive limit means "no limit"; use a sane page + // size in that case so per_page is always valid. + perPage := limit + if perPage <= 0 { + 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) @@ -49,6 +57,7 @@ func (c *Client) GetIssues(owner, repo string, limit int) ([]Issue, error) { var issues []Issue err = json.NewDecoder(resp.Body).Decode(&issues) + nextLink := resp.Header.Get("Link") resp.Body.Close() if err != nil { return nil, err @@ -56,12 +65,23 @@ func (c *Client) GetIssues(owner, repo string, limit int) ([]Issue, error) { allIssues = append(allIssues, issues...) - if len(allIssues) >= limit { + // Stop when the user's limit is reached. + if limit > 0 && len(allIssues) >= limit { allIssues = allIssues[:limit] break } - nextURL = getNextPageURL(resp.Header.Get("Link")) + // Safety fallback: an empty page must stop pagination even if a next + // link is present, preventing an infinite loop against a misbehaving + // API that keeps returning empty pages with a next relation. + if len(issues) == 0 { + break + } + + // Continue strictly based on the Link header's rel="next" relation, + // never on the number of items returned (a sparse page can carry a + // next link with fewer items than requested). + nextURL = getNextPageURL(nextLink) } return allIssues, nil From 9fccf5d4d9de8fcc7e0f0b16e11f41af0d169f66 Mon Sep 17 00:00:00 2001 From: jearthliu Date: Wed, 12 Aug 2026 10:07:26 +0800 Subject: [PATCH 2/3] test: cover empty-page fallback and limit --- workspace/pkg/github/client_test.go | 38 +++++++++++++++++++++++------ 1 file changed, 31 insertions(+), 7 deletions(-) diff --git a/workspace/pkg/github/client_test.go b/workspace/pkg/github/client_test.go index 1bde044..f18e892 100644 --- a/workspace/pkg/github/client_test.go +++ b/workspace/pkg/github/client_test.go @@ -69,22 +69,19 @@ func TestGetIssues_SparsePagination(t *testing.T) { } } -func TestGetIssues_LimitAndZeroItems(t *testing.T) { +func TestGetIssues_Limit(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"`) + w.Header().Set("Link", `<`+server.URL+`/repos/owner/repo/issues?page=2&per_page=3>; rel="next"`) json.NewEncoder(w).Encode(issues) } else { t.Errorf("unexpected request count: %d", requestCount) @@ -98,11 +95,38 @@ 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 (limit reached on first page), got %d", requestCount) } if len(issues) != 3 { t.Errorf("expected 3 issues, got %d", len(issues)) } } + +func TestGetIssues_EmptyPageStops(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") + // Empty page with a next link must NOT be followed — the empty-list + // fallback stops pagination to prevent infinite loops. + 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() + + client := NewClient(server.URL, nil) + issues, err := client.GetIssues("owner", "repo", 0) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if requestCount != 1 { + t.Errorf("expected exactly 1 request (empty page stops pagination), got %d", requestCount) + } + + if len(issues) != 0 { + t.Errorf("expected 0 issues, got %d", len(issues)) + } +} From 17f799d67de9bf997c7ef62e35ab73067a0d91d5 Mon Sep 17 00:00:00 2001 From: jearthliu Date: Wed, 12 Aug 2026 10:10:54 +0800 Subject: [PATCH 3/3] fix: clamp per_page to GitHub 100 limit --- workspace/pkg/github/client.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/workspace/pkg/github/client.go b/workspace/pkg/github/client.go index f41c05f..57391b9 100644 --- a/workspace/pkg/github/client.go +++ b/workspace/pkg/github/client.go @@ -31,11 +31,16 @@ func (c *Client) GetIssues(owner, repo string, limit int) ([]Issue, error) { var allIssues []Issue // Per-page size. A non-positive limit means "no limit"; use a sane page - // size in that case so per_page is always valid. + // size in that case. GitHub caps per_page at 100, so clamp to avoid + // unnecessary round trips (a limit of 5000 would otherwise still fetch + // 100/page but iterate far more pages than needed). perPage := limit if perPage <= 0 { perPage = 100 } + if perPage > 100 { + perPage = 100 + } nextURL := fmt.Sprintf("%s/repos/%s/%s/issues?per_page=%d", c.BaseURL, owner, repo, perPage)