Skip to content

Commit 8becec1

Browse files
jedudenclaude
andauthored
Fix publish-release: find draft via list endpoint, not by-tag (#298)
The release job creates the GitHub release as a draft so assets stay mutable until the final publish. GitHub's GET /releases/tags/{tag} endpoint omits draft releases, so the by-tag lookup always 404'd and publish-release could never find the draft to flip — failing every release with "no GitHub release found for tag". List releases via GET /releases and match on tag_name (drafts are only visible there), following Link-header pagination since a fresh draft is not guaranteed to land on the first page. https://claude.ai/code/session_01F3gFEBvZLjWfAuoqcMNoSZ Co-authored-by: Claude <noreply@anthropic.com>
1 parent 318f1ba commit 8becec1

3 files changed

Lines changed: 122 additions & 27 deletions

File tree

cmd/mdsmith-release/publishrelease_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ func TestRunPublishReleaseFlipsDraft(t *testing.T) {
1616
_, _ = w.Write([]byte(`{"id":42,"draft":false}`))
1717
return
1818
}
19-
_, _ = w.Write([]byte(`{"id":42,"draft":true}`))
19+
_, _ = w.Write([]byte(`[{"id":42,"draft":true,"tag_name":"v1.2.3"}]`))
2020
}))
2121
t.Cleanup(srv.Close)
2222

internal/release/publishrelease.go

Lines changed: 65 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import (
66
"fmt"
77
"io"
88
"net/http"
9-
"net/url"
109
"strings"
1110
"time"
1211
)
@@ -101,35 +100,85 @@ func PublishRelease(opts PublishReleaseOptions) error {
101100
}
102101

103102
type releaseRef struct {
104-
ID int64 `json:"id"`
105-
Draft bool `json:"draft"`
103+
ID int64 `json:"id"`
104+
Draft bool `json:"draft"`
105+
TagName string `json:"tag_name"`
106106
}
107107

108+
// lookupReleaseRef finds the release whose tag_name matches tag by
109+
// walking the list-releases endpoint. The by-tag endpoint
110+
// (GET /releases/tags/{tag}) deliberately omits draft releases, and
111+
// the `release` job creates the release as a draft so its assets stay
112+
// mutable until the final publish — so the draft is only reachable
113+
// through the list endpoint. Pagination is followed via the Link
114+
// header because drafts are not guaranteed to land on the first page.
108115
func lookupReleaseRef(client *http.Client, apiBase, repository, tag, token string) (releaseRef, bool, error) {
109-
u := apiBase + "/repos/" + repository + "/releases/tags/" + url.PathEscape(tag)
116+
next := apiBase + "/repos/" + repository + "/releases?per_page=100"
117+
for next != "" {
118+
rel, found, link, err := lookupReleasePage(client, next, tag, token)
119+
if err != nil {
120+
return releaseRef{}, false, err
121+
}
122+
if found {
123+
return rel, true, nil
124+
}
125+
next = link
126+
}
127+
return releaseRef{}, false, nil
128+
}
129+
130+
func lookupReleasePage(client *http.Client, u, tag, token string) (releaseRef, bool, string, error) {
110131
req, err := newGitHubRequest(http.MethodGet, u, nil, token)
111132
if err != nil {
112-
return releaseRef{}, false, err
133+
return releaseRef{}, false, "", err
113134
}
114135

115136
resp, err := client.Do(req)
116137
if err != nil {
117-
return releaseRef{}, false, err
138+
return releaseRef{}, false, "", err
118139
}
119140
defer func() { _ = resp.Body.Close() }()
120141

121-
switch resp.StatusCode {
122-
case http.StatusOK:
123-
var rel releaseRef
124-
if err := json.NewDecoder(resp.Body).Decode(&rel); err != nil {
125-
return releaseRef{}, false, fmt.Errorf("parse %s: %w", u, err)
142+
if resp.StatusCode != http.StatusOK {
143+
return releaseRef{}, false, "", unexpectedStatus("lookup", u, resp)
144+
}
145+
146+
var rels []releaseRef
147+
if err := json.NewDecoder(resp.Body).Decode(&rels); err != nil {
148+
return releaseRef{}, false, "", fmt.Errorf("parse %s: %w", u, err)
149+
}
150+
for _, rel := range rels {
151+
if rel.TagName == tag {
152+
return rel, true, "", nil
153+
}
154+
}
155+
return releaseRef{}, false, nextPageURL(resp.Header.Get("Link")), nil
156+
}
157+
158+
// nextPageURL extracts the rel="next" target from a GitHub Link
159+
// header, or "" when there is no further page.
160+
func nextPageURL(link string) string {
161+
for _, part := range strings.Split(link, ",") {
162+
segs := strings.Split(part, ";")
163+
if len(segs) < 2 {
164+
continue
165+
}
166+
isNext := false
167+
for _, attr := range segs[1:] {
168+
if strings.TrimSpace(attr) == `rel="next"` {
169+
isNext = true
170+
break
171+
}
172+
}
173+
if !isNext {
174+
continue
126175
}
127-
return rel, true, nil
128-
case http.StatusNotFound:
129-
return releaseRef{}, false, nil
130-
default:
131-
return releaseRef{}, false, unexpectedStatus("lookup", u, resp)
176+
u := strings.TrimSpace(segs[0])
177+
u = strings.TrimPrefix(u, "<")
178+
u = strings.TrimSuffix(u, ">")
179+
return u
132180
}
181+
return ""
133182
}
134183

135184
func patchReleaseDraftFalse(client *http.Client, apiBase, repository string, id int64, token string) error {

internal/release/publishrelease_test.go

Lines changed: 56 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -36,13 +36,15 @@ func TestPublishReleaseFlipsDraftToPublished(t *testing.T) {
3636
defer mu.Unlock()
3737
authHdr = append(authHdr, r.Header.Get("Authorization"))
3838
switch {
39-
case r.Method == http.MethodGet && r.URL.Path == "/repos/jeduden/mdsmith/releases/tags/v1.2.3":
39+
case r.Method == http.MethodGet && r.URL.Path == "/repos/jeduden/mdsmith/releases":
4040
gets++
4141
if gets == 1 {
42-
http.NotFound(w, r)
42+
// The draft has not materialized yet: the list
43+
// endpoint returns other releases but not this tag.
44+
_, _ = fmt.Fprint(w, `[{"id":7,"draft":false,"tag_name":"v1.2.2"}]`)
4345
return
4446
}
45-
_, _ = fmt.Fprint(w, `{"id":42,"draft":true}`)
47+
_, _ = fmt.Fprint(w, `[{"id":42,"draft":true,"tag_name":"v1.2.3"}]`)
4648
case r.Method == http.MethodPatch && r.URL.Path == "/repos/jeduden/mdsmith/releases/42":
4749
patched = true
4850
patchID = r.URL.Path
@@ -76,12 +78,55 @@ func TestPublishReleaseFlipsDraftToPublished(t *testing.T) {
7678
}
7779
}
7880

81+
func TestPublishReleaseFollowsPaginationToFindDraft(t *testing.T) {
82+
var patched bool
83+
var srv *httptest.Server
84+
srv = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
85+
isList := r.Method == http.MethodGet && r.URL.Path == "/repos/jeduden/mdsmith/releases"
86+
page := r.URL.Query().Get("page")
87+
switch {
88+
case isList && page == "":
89+
w.Header().Set("Link",
90+
`<`+srv.URL+`/repos/jeduden/mdsmith/releases?page=2>; rel="next", `+
91+
`<`+srv.URL+`/repos/jeduden/mdsmith/releases?page=2>; rel="last"`)
92+
_, _ = fmt.Fprint(w, `[{"id":1,"draft":false,"tag_name":"v1.2.2"}]`)
93+
case isList && page == "2":
94+
_, _ = fmt.Fprint(w, `[{"id":99,"draft":true,"tag_name":"v1.2.3"}]`)
95+
case r.Method == http.MethodPatch && r.URL.Path == "/repos/jeduden/mdsmith/releases/99":
96+
patched = true
97+
_, _ = fmt.Fprint(w, `{"id":99,"draft":false}`)
98+
default:
99+
t.Errorf("unexpected request %s %s", r.Method, r.URL.String())
100+
http.Error(w, "unexpected", http.StatusTeapot)
101+
}
102+
}))
103+
t.Cleanup(srv.Close)
104+
105+
err := PublishRelease(PublishReleaseOptions{
106+
Repository: "jeduden/mdsmith",
107+
Tag: "v1.2.3",
108+
Token: "t",
109+
APIBaseURL: srv.URL,
110+
})
111+
require.NoError(t, err)
112+
assert.True(t, patched)
113+
}
114+
115+
func TestNextPageURL(t *testing.T) {
116+
hdr := `<https://api.github.com/repositories/1/releases?page=2>; rel="next", ` +
117+
`<https://api.github.com/repositories/1/releases?page=9>; rel="last"`
118+
assert.Equal(t, "https://api.github.com/repositories/1/releases?page=2", nextPageURL(hdr))
119+
120+
assert.Equal(t, "", nextPageURL(""))
121+
assert.Equal(t, "", nextPageURL(`<https://x/p?page=9>; rel="last"`))
122+
}
123+
79124
func TestPublishReleaseAlreadyPublishedIsNoOp(t *testing.T) {
80125
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
81126
if r.Method != http.MethodGet {
82127
t.Errorf("already-published release must not be patched, got %s", r.Method)
83128
}
84-
_, _ = fmt.Fprint(w, `{"id":7,"draft":false}`)
129+
_, _ = fmt.Fprint(w, `[{"id":7,"draft":false,"tag_name":"v1.2.3"}]`)
85130
}))
86131
t.Cleanup(srv.Close)
87132

@@ -96,9 +141,10 @@ func TestPublishReleaseAlreadyPublishedIsNoOp(t *testing.T) {
96141

97142
func TestPublishReleaseMissingAfterRetriesErrors(t *testing.T) {
98143
var calls, sleeps int
99-
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
144+
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
100145
calls++
101-
http.NotFound(w, r)
146+
// The draft for v9.9.9 never appears in the list.
147+
_, _ = fmt.Fprint(w, `[{"id":1,"draft":false,"tag_name":"v9.9.8"}]`)
102148
}))
103149
t.Cleanup(srv.Close)
104150

@@ -151,7 +197,7 @@ func TestPublishReleaseLookupUnexpectedStatusErrors(t *testing.T) {
151197
func TestPublishReleasePatchUnexpectedStatusErrors(t *testing.T) {
152198
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
153199
if r.Method == http.MethodGet {
154-
_, _ = fmt.Fprint(w, `{"id":3,"draft":true}`)
200+
_, _ = fmt.Fprint(w, `[{"id":3,"draft":true,"tag_name":"v1.2.3"}]`)
155201
return
156202
}
157203
w.WriteHeader(http.StatusUnprocessableEntity)
@@ -200,7 +246,7 @@ func TestPublishReleaseUsesDefaultAPIBase(t *testing.T) {
200246
gotURL = r.URL.String()
201247
return &http.Response{
202248
StatusCode: http.StatusOK,
203-
Body: io.NopCloser(strings.NewReader(`{"id":1,"draft":false}`)),
249+
Body: io.NopCloser(strings.NewReader(`[{"id":1,"draft":false,"tag_name":"v1.2.3"}]`)),
204250
Header: make(http.Header),
205251
}, nil
206252
})}
@@ -211,7 +257,7 @@ func TestPublishReleaseUsesDefaultAPIBase(t *testing.T) {
211257
Client: client,
212258
})
213259
require.NoError(t, err)
214-
assert.Equal(t, "https://api.github.com/repos/jeduden/mdsmith/releases/tags/v1.2.3", gotURL)
260+
assert.Equal(t, "https://api.github.com/repos/jeduden/mdsmith/releases?per_page=100", gotURL)
215261
}
216262

217263
func TestPublishReleaseClientDoError(t *testing.T) {
@@ -237,7 +283,7 @@ func TestPublishReleasePatchTransportError(t *testing.T) {
237283
}
238284
return &http.Response{
239285
StatusCode: http.StatusOK,
240-
Body: io.NopCloser(strings.NewReader(`{"id":5,"draft":true}`)),
286+
Body: io.NopCloser(strings.NewReader(`[{"id":5,"draft":true,"tag_name":"v1.2.3"}]`)),
241287
Header: make(http.Header),
242288
}, nil
243289
})}

0 commit comments

Comments
 (0)