Skip to content

Commit 3d6700a

Browse files
authored
fix(github): GetTagForSHA must paginate exhaustively — bounded 20-tag lookup misses old tags (#78)
The pre-extraction bounded version survived in the SDK while Pilot's in-tree client was upgraded to exhaustive pagination (per_page=100, up to 50 pages). A SHA tagged beyond the window made hosts treat already-released commits as untagged, stalling release draining. Caught by Pilot's TestHandleReleasing_ExhaustiveTagDrain during the M7 4d.1 client swap.
1 parent 7b08c81 commit 3d6700a

2 files changed

Lines changed: 56 additions & 8 deletions

File tree

sdk/integrations/github/client.go

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -639,15 +639,26 @@ func (c *Client) ListTags(ctx context.Context, owner, repo string, perPage int)
639639
return result, nil
640640
}
641641

642-
// GetTagForSHA returns the tag name if a tag exists at the given SHA, or empty string if none.
642+
// GetTagForSHA returns the tag name if a tag exists at the given SHA, or empty
643+
// string if none. Paginates exhaustively (per_page=100, up to 50 pages) — a
644+
// bounded lookup misses tags beyond the first page and stalls release draining
645+
// for SHAs tagged long ago (caught by Pilot's TestHandleReleasing_ExhaustiveTagDrain).
643646
func (c *Client) GetTagForSHA(ctx context.Context, owner, repo, sha string) (string, error) {
644-
tags, err := c.ListTags(ctx, owner, repo, 20)
645-
if err != nil {
646-
return "", err
647-
}
648-
for _, tag := range tags {
649-
if tag.Commit.SHA == sha {
650-
return tag.Name, nil
647+
const perPage = 100
648+
const maxPages = 50
649+
for page := 1; page <= maxPages; page++ {
650+
path := fmt.Sprintf("/repos/%s/%s/tags?per_page=%d&page=%d", owner, repo, perPage, page)
651+
var batch []*Tag
652+
if err := c.doRequest(ctx, http.MethodGet, path, nil, &batch); err != nil {
653+
return "", err
654+
}
655+
for _, tag := range batch {
656+
if tag.Commit.SHA == sha {
657+
return tag.Name, nil
658+
}
659+
}
660+
if len(batch) < perPage {
661+
break
651662
}
652663
}
653664
return "", nil

sdk/integrations/github/errors_test.go

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,9 @@ package github
22

33
import (
44
"context"
5+
"encoding/json"
56
"errors"
7+
"fmt"
68
"net/http"
79
"net/http/httptest"
810
"strconv"
@@ -185,3 +187,38 @@ func TestGetOpenSubIssueNumbers_NoNativeLinks(t *testing.T) {
185187
t.Errorf("got (%v, %v), want (nil, false)", numbers, hasLinks)
186188
}
187189
}
190+
191+
func TestGetTagForSHA_Exhaustive(t *testing.T) {
192+
// Target SHA appears on page 2 — a bounded single-page lookup misses it.
193+
page1 := make([]map[string]interface{}, 100)
194+
for i := range page1 {
195+
page1[i] = map[string]interface{}{"name": fmt.Sprintf("v1.0.%d", i), "commit": map[string]string{"sha": fmt.Sprintf("p1sha%03d", i)}}
196+
}
197+
page2 := []map[string]interface{}{
198+
{"name": "v0.9.7-target", "commit": map[string]string{"sha": "deepsha"}},
199+
}
200+
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
201+
w.Header().Set("Content-Type", "application/json")
202+
if r.URL.Query().Get("page") == "2" {
203+
_ = json.NewEncoder(w).Encode(page2)
204+
return
205+
}
206+
_ = json.NewEncoder(w).Encode(page1)
207+
}))
208+
defer srv.Close()
209+
210+
c := NewClientWithBaseURL(testutil.FakeGitHubToken, srv.URL)
211+
name, err := c.GetTagForSHA(context.Background(), "o", "r", "deepsha")
212+
if err != nil {
213+
t.Fatalf("GetTagForSHA: %v", err)
214+
}
215+
if name != "v0.9.7-target" {
216+
t.Errorf("tag = %q, want v0.9.7-target (page-2 tag must be found)", name)
217+
}
218+
219+
// Absent SHA terminates on the short page without error.
220+
name, err = c.GetTagForSHA(context.Background(), "o", "r", "nosuch")
221+
if err != nil || name != "" {
222+
t.Errorf("absent SHA: got (%q, %v), want (\"\", nil)", name, err)
223+
}
224+
}

0 commit comments

Comments
 (0)