Consider merging this with the if statement added above, so it's a little easier to see that this if statement deals with the same err value from line 81:

if _, ok := err.(*AcceptedError); ok {
	...
} else if err != nil {
	...
}

I think it's fine keeping them separate. They are very different code paths; the AcceptedError case is only nominally an error.

Doing this makes sense, but I don't think it's the right place.

We don't want it to run in all cases when err != nil. If you look at the code of CheckResponse, it does the following things:

1. if status code is accepted, it immediately returns &AcceptedError{}
2. if status code is in 2xx range, it returns nil
3. it reads the response body into a byte slice (data, err := ioutil.ReadAll(r.Body))
4. it returns some specific error type or a general &ErrorResponse{Response: r}

We don't want this code to run after ioutil.ReadAll(r.Body) already happened in CheckResponse.

I think a good fix is to move this code into CheckResponse's first case:

func CheckResponse(r *http.Response) error {
	if r.StatusCode == http.StatusAccepted {
		// Since Do closes the original response body upon returning,
		// read its contents now so the caller can use it,
		// and wrap it such that it doesn't matter if they close it.
		body, _ := ioutil.ReadAll(r.Body)
		r.Body = ioutil.NopCloser(bytes.NewReader(body))
		return &AcceptedError{}
	}
	...
}

This gives me a followup idea. Instead of using r.Body as a way of transmitting the body from here into CreateFork, we can put it inside AcceptedError:

type AcceptedError struct {
	body []byte
}

func CheckResponse(r *http.Response) error {
	if r.StatusCode == http.StatusAccepted {
		// Since Do closes the original response body upon returning,
		// read its contents now so that CreateFork can use it.
		body, _ := ioutil.ReadAll(r.Body)
		return &AcceptedError{body: body}
	}
	...
}

Then CreateFork code becomes more clear:

 	fork := new(Repository)
 	resp, err := s.client.Do(ctx, req, fork)
+	if ae, ok := err.(*AcceptedError); ok {
+		json.NewDecoder(bytes.NewReader(ae.body)).Decode(fork)
+		return fork, resp, err
+	}
 	if err != nil {

I think this has the benefits of not changing externally visible behavior of the library (edit: actually, it still changes CheckResponse slightly), and it's easier to read and be sure is correct. If resp.Body is modified as currently done, then all other endpoints which don't specifically handle AcceptedError will be returning a resp with a body. Users may start to rely on that, and we don't neccessary want to promise it'll always work that way.

In the future, we can consider either exporting AcceptedError.body so users of this package can use it, or implement a different solution if we find a better one.

What do you think about this idea?

Doing that in CheckResponse merely hides the issue from the non-trivial control flow in Do (which is where the body closing is happening). Right now, CheckResponse doesn't touch the body in the HTTP 202 case, just like in any of the other 2xx cases. Your suggestion would mean that Do needs to know that CheckResponse does that transformation and therefore closing the body is okay.

Now Do currently handles all the non-error decoding, so perhaps it should handle this case too. It has the destination object there already. Then the body closing is moot. Let me take that approach; I think it may be the best option.

In regard to your follow-up idea: I think that's unnecessary. The AcceptedError flows are generally all places where the response body is some subset of the regular response (that's why it is a 2xx code rather than a different code), so forcing the callers to do the parsing seems like unnecessary work. I think my latest revision (having Do do the decoding when there's an AcceptedError) is the simplest option, and it still preserves existing code behaviour unless the caller is doing something truly odd.