cli · williammartin · Jul 28, 2026
@@ -2,16 +2,93 @@ package add

 import (
 	"net/http"
+	"strings"
 	"testing"

 	"github.com/MakeNowJust/heredoc"
+	"github.com/cli/cli/v2/api"
 	"github.com/cli/cli/v2/internal/config"
 	"github.com/cli/cli/v2/internal/gh"
 	"github.com/cli/cli/v2/pkg/httpmock"
 	"github.com/cli/cli/v2/pkg/iostreams"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )

+func Test_gpgKeyUploadScopesMissing(t *testing.T) {
+	reg := &httpmock.Registry{}
+	defer reg.Verify(t)
+	reg.Register(
+		httpmock.REST("POST", "user/gpg_keys"),
+		httpmock.StatusStringResponse(http.StatusNotFound, `{"message":"Not Found"}`),
+	)
+
+	err := gpgKeyUpload(&http.Client{Transport: reg}, "github.com", strings.NewReader("-----BEGIN PGP PUBLIC KEY BLOCK-----"), "")
+
+	require.Same(t, errScopesMissing, err)
+}
+
+func Test_gpgKeyUploadDuplicateKeyBeforeWrongFormat(t *testing.T) {
+	reg := &httpmock.Registry{}
+	defer reg.Verify(t)
+	reg.Register(
+		httpmock.REST("POST", "user/gpg_keys"),
+		httpmock.WithHeader(httpmock.StatusStringResponse(http.StatusUnprocessableEntity, `{
+			"message": "Validation Failed",
+			"errors": [{
+				"resource": "GpgKey",
+				"code": "custom",
+				"field": "key_id",
+				"message": "key_id already exists"
+			}]
+		}`), "Content-Type", "application/json"),
+	)
+
+	err := gpgKeyUpload(&http.Client{Transport: reg}, "github.com", strings.NewReader("binary-key"), "")
+
+	require.Same(t, errDuplicateKey, err)
+}
+
+func Test_gpgKeyUploadWrongFormat(t *testing.T) {
+	reg := &httpmock.Registry{}
+	defer reg.Verify(t)
+	reg.Register(
+		httpmock.REST("POST", "user/gpg_keys"),
+		httpmock.StatusStringResponse(http.StatusUnprocessableEntity, `{
+			"message": "Validation Failed",
+			"errors": [{
+				"resource": "GpgKey",
+				"code": "custom",
+				"message": "We got an error doing that."
+			}]
+		}`),
+	)
+
+	err := gpgKeyUpload(&http.Client{Transport: reg}, "github.com", strings.NewReader("binary-key"), "")
+
+	require.Same(t, errWrongFormat, err)
+}
+
+func Test_gpgKeyUploadHTTPError(t *testing.T) {
+	reg := &httpmock.Registry{}
+	defer reg.Verify(t)
+	reg.Register(
+		httpmock.REST("POST", "user/gpg_keys"),
+		httpmock.WithHeader(
+			httpmock.StatusStringResponse(http.StatusInternalServerError, `{"message":"Internal Server Error"}`),
+			"Content-Type", "application/json",
+		),
+	)
+
+	err := gpgKeyUpload(&http.Client{Transport: reg}, "github.com", strings.NewReader("-----BEGIN PGP PUBLIC KEY BLOCK-----"), "")
+
+	var httpErr api.HTTPError
+	require.ErrorAs(t, err, &httpErr)
+	assert.Equal(t, http.StatusInternalServerError, httpErr.StatusCode)
+	assert.Contains(t, err.Error(), "HTTP 500")
+	assert.EqualError(t, err, "HTTP 500: Internal Server Error (https://api.github.com/user/gpg_keys)")
+}
+
 func Test_runAdd(t *testing.T) {
 	tests := []struct {
 		name       string
@@ -28,7 +105,7 @@ func Test_runAdd(t *testing.T) {
 			httpStubs: func(reg *httpmock.Registry) {
 				reg.Register(
 					httpmock.REST("POST", "user/gpg_keys"),
-					httpmock.RESTPayload(200, ``, func(payload map[string]interface{}) {
+					httpmock.RESTPayload(200, `{}`, func(payload map[string]interface{}) {
 						assert.Contains(t, payload, "armored_public_key")
 						assert.NotContains(t, payload, "title")
 					}))
@@ -44,7 +121,7 @@ func Test_runAdd(t *testing.T) {
 			httpStubs: func(reg *httpmock.Registry) {
 				reg.Register(
 					httpmock.REST("POST", "user/gpg_keys"),
-					httpmock.RESTPayload(200, ``, func(payload map[string]interface{}) {
+					httpmock.RESTPayload(200, `{}`, func(payload map[string]interface{}) {
 						assert.Contains(t, payload, "armored_public_key")
 						assert.Contains(t, payload, "name")
 					}))

@@ -8,16 +8,13 @@ import (
 	"net/http"

 	"github.com/cli/cli/v2/api"
-	"github.com/cli/cli/v2/internal/ghinstance"
 )

 var errScopesMissing = errors.New("insufficient OAuth scopes")
 var errDuplicateKey = errors.New("key already exists")
 var errWrongFormat = errors.New("key in wrong format")

 func gpgKeyUpload(httpClient *http.Client, hostname string, keyFile io.Reader, title string) error {
-	url := ghinstance.RESTPrefix(hostname) + "user/gpg_keys"
-
 	keyBytes, err := io.ReadAll(keyFile)
 	if err != nil {
 		return err
@@ -35,36 +32,29 @@ func gpgKeyUpload(httpClient *http.Client, hostname string, keyFile io.Reader, t
 		return err
 	}

-	req, err := http.NewRequest("POST", url, bytes.NewBuffer(payloadBytes))
-	if err != nil {
-		return err
-	}
-
-	resp, err := httpClient.Do(req)
+	// TODO(api-client-rollout)
+	// This line of code is part of a mechanical roll out of the api client.
+	// As a follow up, consider whether the api client can be injected to this call site, rather than constructed
+	apiClient := api.NewClientFromHTTP(httpClient)
+	err = apiClient.REST(hostname, "POST", "user/gpg_keys", bytes.NewBuffer(payloadBytes), nil)
 	if err != nil {
-		return err
-	}
-	defer resp.Body.Close()
-
-	if resp.StatusCode == 404 {
-		return errScopesMissing
-	} else if resp.StatusCode > 299 {
-		err := api.HandleHTTPError(resp)
 		var httpError api.HTTPError
 		if errors.As(err, &httpError) {
+			if httpError.StatusCode == 404 {
+				return errScopesMissing
+			}
 			for _, e := range httpError.Errors {
-				if resp.StatusCode == 422 && e.Field == "key_id" && e.Message == "key_id already exists" {
+				if httpError.StatusCode == 422 && e.Field == "key_id" && e.Message == "key_id already exists" {
 					return errDuplicateKey
 				}
 			}
-		}
-		if resp.StatusCode == 422 && !isGpgKeyArmored(keyBytes) {
-			return errWrongFormat
+			if httpError.StatusCode == 422 && !isGpgKeyArmored(keyBytes) {
+				return errWrongFormat
+			}
 		}
 		return err
 	}

-	_, _ = io.Copy(io.Discard, resp.Body)
 	return nil
 }


@@ -3,19 +3,63 @@ package delete
 import (
 	"bytes"
 	"net/http"
+	"net/url"
 	"testing"

+	"github.com/cli/cli/v2/api"
 	"github.com/cli/cli/v2/internal/config"
 	"github.com/cli/cli/v2/internal/gh"
 	"github.com/cli/cli/v2/internal/prompter"
 	"github.com/cli/cli/v2/pkg/cmdutil"
 	"github.com/cli/cli/v2/pkg/httpmock"
 	"github.com/cli/cli/v2/pkg/iostreams"
-	"github.com/cli/go-gh/v2/pkg/api"
+	ghAPI "github.com/cli/go-gh/v2/pkg/api"
 	"github.com/google/shlex"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )

+func Test_deleteGPGKeyHTTPError(t *testing.T) {
+	reg := &httpmock.Registry{}
+	defer reg.Verify(t)
+	reg.Register(
+		httpmock.REST("DELETE", "user/gpg_keys/123"),
+		httpmock.WithHeader(
+			httpmock.StatusStringResponse(http.StatusInternalServerError, `{"message":"Internal Server Error"}`),
+			"Content-Type", "application/json",
+		),
+	)
+
+	err := deleteGPGKey(&http.Client{Transport: reg}, "github.com", "123")
+
+	var httpErr api.HTTPError
+	require.ErrorAs(t, err, &httpErr)
+	assert.Equal(t, http.StatusInternalServerError, httpErr.StatusCode)
+	assert.Contains(t, err.Error(), "HTTP 500")
+	assert.EqualError(t, err, "HTTP 500: Internal Server Error (https://api.github.com/user/gpg_keys/123)")
+}
+
+func Test_getGPGKeysHTTPError(t *testing.T) {
+	reg := &httpmock.Registry{}
+	defer reg.Verify(t)
+	reg.Register(
+		httpmock.QueryMatcher("GET", "user/gpg_keys", url.Values{"per_page": []string{"100"}}),
+		httpmock.WithHeader(
+			httpmock.StatusStringResponse(http.StatusInternalServerError, `{"message":"Internal Server Error"}`),
+			"Content-Type", "application/json",
+		),
+	)
+
+	keys, err := getGPGKeys(&http.Client{Transport: reg}, "github.com")
+
+	assert.Nil(t, keys)
+	var httpErr api.HTTPError
+	require.ErrorAs(t, err, &httpErr)
+	assert.Equal(t, http.StatusInternalServerError, httpErr.StatusCode)
+	assert.Contains(t, err.Error(), "HTTP 500")
+	assert.EqualError(t, err, "HTTP 500: Internal Server Error (https://api.github.com/user/gpg_keys?per_page=100)")
+}
+
 func TestNewCmdDelete(t *testing.T) {
 	tests := []struct {
 		name       string
@@ -177,7 +221,7 @@ func Test_deleteRun(t *testing.T) {
 			opts: DeleteOptions{KeyID: "ABC123", Confirmed: true},
 			httpStubs: func(reg *httpmock.Registry) {
 				reg.Register(httpmock.REST("GET", "user/gpg_keys"), httpmock.StatusStringResponse(200, keysResp))
-				reg.Register(httpmock.REST("DELETE", "user/gpg_keys/123"), httpmock.JSONErrorResponse(404, api.HTTPError{
+				reg.Register(httpmock.REST("DELETE", "user/gpg_keys/123"), httpmock.JSONErrorResponse(404, ghAPI.HTTPError{
 					StatusCode: 404,
 					Message:    "GPG key 123 not found",
 				}))

@@ -1,13 +1,10 @@
 package delete

 import (
-	"encoding/json"
 	"fmt"
-	"io"
 	"net/http"

 	"github.com/cli/cli/v2/api"
-	"github.com/cli/cli/v2/internal/ghinstance"
 )

 type gpgKey struct {
@@ -16,53 +13,21 @@ type gpgKey struct {
 }

 func deleteGPGKey(httpClient *http.Client, host, id string) error {
-	url := fmt.Sprintf("%suser/gpg_keys/%s", ghinstance.RESTPrefix(host), id)
-	req, err := http.NewRequest("DELETE", url, nil)
-	if err != nil {
-		return err
-	}
-
-	resp, err := httpClient.Do(req)
-	if err != nil {
-		return err
-	}
-	defer resp.Body.Close()
-
-	if resp.StatusCode > 299 {
-		return api.HandleHTTPError(resp)
-	}
-
-	return nil
+	path := fmt.Sprintf("user/gpg_keys/%s", id)
+	// TODO(api-client-rollout)
+	// This line of code is part of a mechanical roll out of the api client.
+	// As a follow up, consider whether the api client can be injected to this call site, rather than constructed
+	return api.NewClientFromHTTP(httpClient).REST(host, "DELETE", path, nil, nil)
 }

 func getGPGKeys(httpClient *http.Client, host string) ([]gpgKey, error) {
-	resource := "user/gpg_keys"
-	url := fmt.Sprintf("%s%s?per_page=%d", ghinstance.RESTPrefix(host), resource, 100)
-	req, err := http.NewRequest("GET", url, nil)
-	if err != nil {
-		return nil, err
-	}
-
-	resp, err := httpClient.Do(req)
-	if err != nil {
-		return nil, err
-	}
-	defer resp.Body.Close()
-
-	if resp.StatusCode > 299 {
-		return nil, api.HandleHTTPError(resp)
-	}
-
-	b, err := io.ReadAll(resp.Body)
-	if err != nil {
-		return nil, err
-	}
-
 	var keys []gpgKey
-	err = json.Unmarshal(b, &keys)
+	// TODO(api-client-rollout)
+	// This line of code is part of a mechanical roll out of the api client.
+	// As a follow up, consider whether the api client can be injected to this call site, rather than constructed
+	err := api.NewClientFromHTTP(httpClient).REST(host, "GET", "user/gpg_keys?per_page=100", nil, &keys)
 	if err != nil {
 		return nil, err
 	}
-
 	return keys, nil
 }
@@ -1,16 +1,13 @@
 package list

 import (
-	"encoding/json"
 	"errors"
 	"fmt"
-	"io"
 	"net/http"
 	"strings"
 	"time"

 	"github.com/cli/cli/v2/api"
-	"github.com/cli/cli/v2/internal/ghinstance"
 )

 var errScopes = errors.New("insufficient OAuth scopes")
@@ -42,32 +39,18 @@ func userKeys(httpClient *http.Client, host, userHandle string) ([]gpgKey, error
 	if userHandle != "" {
 		resource = fmt.Sprintf("users/%s/gpg_keys", userHandle)
 	}
-	url := fmt.Sprintf("%s%s?per_page=%d", ghinstance.RESTPrefix(host), resource, 100)
-	req, err := http.NewRequest("GET", url, nil)
-	if err != nil {
-		return nil, err
-	}
-
-	resp, err := httpClient.Do(req)
-	if err != nil {
-		return nil, err
-	}
-	defer resp.Body.Close()
-
-	if resp.StatusCode == 404 {
-		return nil, errScopes
-	} else if resp.StatusCode > 299 {
-		return nil, api.HandleHTTPError(resp)
-	}
-
-	b, err := io.ReadAll(resp.Body)
-	if err != nil {
-		return nil, err
-	}

+	path := fmt.Sprintf("%s?per_page=%d", resource, 100)
 	var keys []gpgKey
-	err = json.Unmarshal(b, &keys)
+	// TODO(api-client-rollout)
+	// This line of code is part of a mechanical roll out of the api client.
+	// As a follow up, consider whether the api client can be injected to this call site, rather than constructed
+	err := api.NewClientFromHTTP(httpClient).REST(host, "GET", path, nil, &keys)
 	if err != nil {
+		var httpErr api.HTTPError
+		if errors.As(err, &httpErr) && httpErr.StatusCode == 404 {
+			return nil, errScopes
+		}
 		return nil, err
 	}