Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 47 additions & 11 deletions internal/github/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,10 @@ type Options struct {
}

type ListIssuesOptions struct {
State string
Since string
Limit int
State string
Since string
Limit int
ExpectedTotal int
}

type RequestError struct {
Expand Down Expand Up @@ -113,33 +114,37 @@ func (c *Client) ListRepositoryIssues(ctx context.Context, owner, repo string, o
values.Set("since", options.Since)
}
path := fmt.Sprintf("/repos/%s/%s/issues?%s", pathEscape(owner), pathEscape(repo), values.Encode())
return c.paginate(ctx, path, options.Limit, reporter)
return c.paginate(ctx, path, options.Limit, options.ExpectedTotal, reporter)
}

func (c *Client) ListIssueComments(ctx context.Context, owner, repo string, number int, reporter Reporter) ([]map[string]any, error) {
path := fmt.Sprintf("/repos/%s/%s/issues/%d/comments?per_page=100", pathEscape(owner), pathEscape(repo), number)
return c.paginate(ctx, path, 0, reporter)
return c.paginate(ctx, path, 0, 0, reporter)
}

func (c *Client) ListPullReviews(ctx context.Context, owner, repo string, number int, reporter Reporter) ([]map[string]any, error) {
path := fmt.Sprintf("/repos/%s/%s/pulls/%d/reviews?per_page=100", pathEscape(owner), pathEscape(repo), number)
return c.paginate(ctx, path, 0, reporter)
return c.paginate(ctx, path, 0, 0, reporter)
}

func (c *Client) ListPullReviewComments(ctx context.Context, owner, repo string, number int, reporter Reporter) ([]map[string]any, error) {
path := fmt.Sprintf("/repos/%s/%s/pulls/%d/comments?per_page=100", pathEscape(owner), pathEscape(repo), number)
return c.paginate(ctx, path, 0, reporter)
return c.paginate(ctx, path, 0, 0, reporter)
}

func (c *Client) ListPullFiles(ctx context.Context, owner, repo string, number int, reporter Reporter) ([]map[string]any, error) {
path := fmt.Sprintf("/repos/%s/%s/pulls/%d/files?per_page=100", pathEscape(owner), pathEscape(repo), number)
return c.paginate(ctx, path, 0, reporter)
return c.paginate(ctx, path, 0, 0, reporter)
}

func (c *Client) paginate(ctx context.Context, firstPath string, limit int, reporter Reporter) ([]map[string]any, error) {
func (c *Client) paginate(ctx context.Context, firstPath string, limit int, expectedItems int, reporter Reporter) ([]map[string]any, error) {
var out []map[string]any
nextPath := firstPath
page := 0
totalPages := 0
if expectedItems > 0 {
totalPages = (expectedItems + 99) / 100
}
for nextPath != "" {
page++
var rows []map[string]any
Expand All @@ -156,11 +161,22 @@ func (c *Client) paginate(ctx context.Context, firstPath string, limit int, repo
rows = rows[:limit-len(out)]
}
out = append(out, rows...)
reporter.Printf("[github] page %d fetched count=%d accumulated=%d", page, len(rows), len(out))
linkHeader := resp.Header.Get("Link")
if last := lastPage(linkHeader); last > totalPages {
totalPages = last
}
if totalPages > 0 && page > totalPages {
totalPages = page
}
if totalPages > 0 {
reporter.Printf("[github] page %d/%d fetched count=%d accumulated=%d", page, totalPages, len(rows), len(out))
} else {
reporter.Printf("[github] page %d fetched count=%d accumulated=%d", page, len(rows), len(out))
}
if limit > 0 && len(out) >= limit {
break
}
nextPath = nextPage(resp.Header.Get("Link"))
nextPath = nextPage(linkHeader)
if nextPath != "" && c.pageDelay > 0 {
select {
case <-ctx.Done():
Expand Down Expand Up @@ -231,6 +247,26 @@ func nextPage(linkHeader string) string {
return ""
}

func lastPage(linkHeader string) int {
for _, part := range strings.Split(linkHeader, ",") {
sections := strings.Split(part, ";")
if len(sections) < 2 {
continue
}
if strings.TrimSpace(sections[1]) != `rel="last"` {
continue
}
rawURL := strings.Trim(strings.TrimSpace(sections[0]), "<>")
parsed, err := url.Parse(rawURL)
if err != nil {
return 0
}
page, _ := strconv.Atoi(parsed.Query().Get("page"))
return page
}
return 0
}

func pathEscape(value string) string {
return url.PathEscape(value)
}
Expand Down
85 changes: 83 additions & 2 deletions internal/github/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ func TestListRepositoryIssuesPaginatesAndLimits(t *testing.T) {
}
switch r.URL.Query().Get("page") {
case "":
w.Header().Set("Link", `<`+serverURL(r)+`?page=2>; rel="next"`)
w.Header().Set("Link", `<`+serverURL(r)+`?page=2>; rel="next", <`+serverURL(r)+`?page=2>; rel="last"`)
_ = json.NewEncoder(w).Encode([]map[string]any{{"number": 1}, {"number": 2}})
case "2":
_ = json.NewEncoder(w).Encode([]map[string]any{{"number": 3}})
Expand All @@ -26,8 +26,10 @@ func TestListRepositoryIssuesPaginatesAndLimits(t *testing.T) {
}))
defer server.Close()

var messages []string
reporter := Reporter(func(message string) { messages = append(messages, message) })
client := New(Options{Token: "token", BaseURL: server.URL, PageDelay: -1})
rows, err := client.ListRepositoryIssues(context.Background(), "openclaw", "gitcrawl", ListIssuesOptions{Limit: 3}, nil)
rows, err := client.ListRepositoryIssues(context.Background(), "openclaw", "gitcrawl", ListIssuesOptions{Limit: 3}, reporter)
if err != nil {
t.Fatalf("list issues: %v", err)
}
Expand All @@ -37,6 +39,76 @@ func TestListRepositoryIssuesPaginatesAndLimits(t *testing.T) {
if intValue(rows[2]["number"]) != 3 {
t.Fatalf("last number: %#v", rows[2]["number"])
}
joined := strings.Join(messages, "\n")
if !strings.Contains(joined, "page 1/2 fetched") || !strings.Contains(joined, "page 2/2 fetched") {
t.Fatalf("expected page X/Y log lines, got:\n%s", joined)
}
}

func TestListRepositoryIssuesUsesExpectedTotalWhenNoLastLink(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
switch r.URL.Query().Get("page") {
case "":
// Cursor-style pagination: only "next", no "last".
w.Header().Set("Link", `<`+serverURL(r)+`?page=2>; rel="next"`)
rows := make([]map[string]any, 100)
for i := range rows {
rows[i] = map[string]any{"number": i + 1}
}
_ = json.NewEncoder(w).Encode(rows)
case "2":
rows := make([]map[string]any, 50)
for i := range rows {
rows[i] = map[string]any{"number": 100 + i + 1}
}
_ = json.NewEncoder(w).Encode(rows)
default:
t.Fatalf("unexpected page: %s", r.URL.RawQuery)
}
}))
defer server.Close()

var messages []string
reporter := Reporter(func(message string) { messages = append(messages, message) })
client := New(Options{BaseURL: server.URL, PageDelay: -1})
rows, err := client.ListRepositoryIssues(context.Background(), "openclaw", "gitcrawl", ListIssuesOptions{ExpectedTotal: 150}, reporter)
if err != nil {
t.Fatalf("list issues: %v", err)
}
if len(rows) != 150 {
t.Fatalf("rows: got %d want 150", len(rows))
}
joined := strings.Join(messages, "\n")
if !strings.Contains(joined, "page 1/2 fetched") || !strings.Contains(joined, "page 2/2 fetched") {
t.Fatalf("expected page X/Y log lines from hint, got:\n%s", joined)
}
}

func TestPaginateRaisesTotalWhenActualExceedsHint(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
switch r.URL.Query().Get("page") {
case "":
w.Header().Set("Link", `<`+serverURL(r)+`?page=2>; rel="next"`)
_ = json.NewEncoder(w).Encode([]map[string]any{{"number": 1}})
case "2":
_ = json.NewEncoder(w).Encode([]map[string]any{{"number": 2}})
default:
t.Fatalf("unexpected page: %s", r.URL.RawQuery)
}
}))
defer server.Close()

var messages []string
reporter := Reporter(func(message string) { messages = append(messages, message) })
client := New(Options{BaseURL: server.URL, PageDelay: -1})
// Hint underestimates (1 page) but the API actually returns 2.
if _, err := client.ListRepositoryIssues(context.Background(), "o", "r", ListIssuesOptions{ExpectedTotal: 1}, reporter); err != nil {
t.Fatalf("list: %v", err)
}
joined := strings.Join(messages, "\n")
if !strings.Contains(joined, "page 2/2 fetched") {
t.Fatalf("expected total to be raised to actual page count, got:\n%s", joined)
}
}

func TestRequestErrorIncludesStatus(t *testing.T) {
Expand Down Expand Up @@ -127,6 +199,15 @@ func TestNextPageAndReporterBranches(t *testing.T) {
if got := nextPage(`<bad-url>; rel="last"`); got != "" {
t.Fatalf("bad next page = %q", got)
}
if got := lastPage(header); got != 9 {
t.Fatalf("last page = %d", got)
}
if got := lastPage(`<https://api.github.test/x?page=3>; rel="next"`); got != 0 {
t.Fatalf("last page without rel=last = %d", got)
}
if got := lastPage(`<%zz>; rel="last"`); got != 0 {
t.Fatalf("last page bad url = %d", got)
}
var messages []string
Reporter(func(message string) { messages = append(messages, message) }).Printf("hello %d", 1)
if len(messages) != 1 || messages[0] != "hello 1" {
Expand Down
21 changes: 18 additions & 3 deletions internal/syncer/syncer.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,9 +103,10 @@ func (s *Syncer) Sync(ctx context.Context, options Options) (Stats, error) {
} else {
var err error
rows, err = s.client.ListRepositoryIssues(ctx, options.Owner, options.Repo, gh.ListIssuesOptions{
State: state,
Since: since,
Limit: options.Limit,
State: state,
Since: since,
Limit: options.Limit,
ExpectedTotal: expectedIssueTotal(repoRaw, state, since, options.Limit),
}, options.Reporter)
if err != nil {
return Stats{}, err
Expand Down Expand Up @@ -435,6 +436,20 @@ func jsonID(value any) string {
}
}

func expectedIssueTotal(repoRaw map[string]any, state, since string, limit int) int {
if state != "open" || since != "" {
return 0
}
count := intValue(repoRaw["open_issues_count"])
if count <= 0 {
return 0
}
if limit > 0 && limit < count {
return limit
}
return count
}

func intValue(value any) int {
switch typed := value.(type) {
case float64:
Expand Down
27 changes: 27 additions & 0 deletions internal/syncer/syncer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -455,6 +455,33 @@ func TestSyncOpenSinceAppliesClosedOverlapSweep(t *testing.T) {
}
}

func TestExpectedIssueTotal(t *testing.T) {
cases := []struct {
name string
repo map[string]any
state string
since string
limit int
want int
}{
{name: "open no filters", repo: map[string]any{"open_issues_count": float64(666)}, state: "open", want: 666},
{name: "open with limit below count", repo: map[string]any{"open_issues_count": float64(666)}, state: "open", limit: 100, want: 100},
{name: "open with limit above count", repo: map[string]any{"open_issues_count": float64(50)}, state: "open", limit: 200, want: 50},
{name: "open with since", repo: map[string]any{"open_issues_count": float64(666)}, state: "open", since: "2026-04-26T00:00:00Z", want: 0},
{name: "closed state", repo: map[string]any{"open_issues_count": float64(666)}, state: "closed", want: 0},
{name: "all state", repo: map[string]any{"open_issues_count": float64(666)}, state: "all", want: 0},
{name: "missing count", repo: map[string]any{}, state: "open", want: 0},
{name: "zero count", repo: map[string]any{"open_issues_count": float64(0)}, state: "open", want: 0},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
if got := expectedIssueTotal(tc.repo, tc.state, tc.since, tc.limit); got != tc.want {
t.Fatalf("expectedIssueTotal = %d, want %d", got, tc.want)
}
})
}
}

func TestMappingHelperBranches(t *testing.T) {
if got := jsonID("abc"); got != "abc" {
t.Fatalf("string json id = %q", got)
Expand Down