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
6 changes: 6 additions & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,12 @@ go test -tags acceptance ./acceptance # Acceptance tests
make lint # golangci-lint (same as CI)
```

**Before committing, ensure both tests and linter pass:**
```bash
go test ./...
make lint
```

## Architecture

Entry point: `cmd/gh/main.go``internal/ghcmd.Main()``pkg/cmd/root.NewCmdRoot()`.
Expand Down
16 changes: 5 additions & 11 deletions internal/authflow/flow.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ func AuthFlow(httpClient *http.Client, oauthHost string, IO *iostreams.IOStreams
return "", "", err
}

userLogin, err := getViewer(oauthHost, token.Token, IO.ErrOut)
userLogin, err := getViewer(httpClient, oauthHost, token.Token)
if err != nil {
return "", "", err
}
Expand All @@ -123,16 +123,10 @@ func (c cfg) ActiveToken(hostname string) (string, string) {
return c.token, "oauth_token"
}

func getViewer(hostname, token string, logWriter io.Writer) (string, error) {
opts := api.HTTPClientOptions{
Config: cfg{token: token},
Log: logWriter,
}
client, err := api.NewHTTPClient(opts)
if err != nil {
return "", err
}
return api.CurrentLoginName(api.NewClientFromHTTP(client), hostname)
func getViewer(httpClient *http.Client, hostname, token string) (string, error) {
authedClient := *httpClient
authedClient.Transport = api.AddAuthTokenHeader(httpClient.Transport, cfg{token: token})
return api.CurrentLoginName(api.NewClientFromHTTP(&authedClient), hostname)
}

func waitForEnter(r io.Reader) error {
Expand Down
37 changes: 37 additions & 0 deletions internal/authflow/flow_test.go
Original file line number Diff line number Diff line change
@@ -1,11 +1,48 @@
package authflow

import (
"bytes"
"io"
"net/http"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func Test_getViewer_leavesUserAgent(t *testing.T) {
var receivedUA string
var receivedAuth string

plainClient := &http.Client{
Transport: &roundTripper{roundTrip: func(req *http.Request) (*http.Response, error) {
receivedUA = req.Header.Get("User-Agent")
receivedAuth = req.Header.Get("Authorization")

return &http.Response{
StatusCode: 200,
Header: http.Header{"Content-Type": []string{"application/json"}},
Body: io.NopCloser(bytes.NewBufferString(`{"data":{"viewer":{"login":"monalisa"}}}`)),
Request: req,
}, nil
}},
}

login, err := getViewer(plainClient, "github.com", "test-token")
require.NoError(t, err)
assert.Equal(t, "monalisa", login)
assert.Empty(t, receivedUA, "User-Agent header should be left unset so that downstream transports can set it")
assert.Equal(t, "token test-token", receivedAuth)
}

type roundTripper struct {
roundTrip func(*http.Request) (*http.Response, error)
}

func (t *roundTripper) RoundTrip(req *http.Request) (*http.Response, error) {
return t.roundTrip(req)
}

func Test_getCallbackURI(t *testing.T) {
tests := []struct {
name string
Expand Down
25 changes: 14 additions & 11 deletions pkg/cmd/api/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,13 @@ const (
)

type ApiOptions struct {
AppVersion string
BaseRepo func() (ghrepo.Interface, error)
Branch func() (string, error)
Config func() (gh.Config, error)
HttpClient func() (*http.Client, error)
IO *iostreams.IOStreams
AppVersion string
InvokingAgent string
BaseRepo func() (ghrepo.Interface, error)
Branch func() (string, error)
Config func() (gh.Config, error)
HttpClient func() (*http.Client, error)
IO *iostreams.IOStreams

Hostname string
RequestMethod string
Expand All @@ -62,11 +63,12 @@ type ApiOptions struct {

func NewCmdApi(f *cmdutil.Factory, runF func(*ApiOptions) error) *cobra.Command {
opts := ApiOptions{
AppVersion: f.AppVersion,
BaseRepo: f.BaseRepo,
Branch: f.Branch,
Config: f.Config,
IO: f.IOStreams,
AppVersion: f.AppVersion,
InvokingAgent: f.InvokingAgent,
BaseRepo: f.BaseRepo,
Branch: f.Branch,
Config: f.Config,
IO: f.IOStreams,
}

cmd := &cobra.Command{
Expand Down Expand Up @@ -385,6 +387,7 @@ func apiRun(opts *ApiOptions) error {
}
opts := api.HTTPClientOptions{
AppVersion: opts.AppVersion,
InvokingAgent: opts.InvokingAgent,
CacheTTL: opts.CacheTTL,
Config: cfg.Authentication(),
EnableCache: opts.CacheTTL > 0,
Expand Down
30 changes: 30 additions & 0 deletions pkg/cmd/api/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1388,6 +1388,36 @@ func Test_apiRun_cache(t *testing.T) {
assert.Equal(t, "", stderr.String(), "stderr")
}

func Test_apiRun_invokingAgent(t *testing.T) {
var receivedUA string
s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
receivedUA = r.Header.Get("User-Agent")
w.WriteHeader(http.StatusNoContent)
}))
t.Cleanup(s.Close)

ios, _, _, _ := iostreams.Test()
options := ApiOptions{
IO: ios,
AppVersion: "1.2.3",
InvokingAgent: "copilot-cli",
Config: func() (gh.Config, error) {
return &ghmock.ConfigMock{
AuthenticationFunc: func() gh.AuthConfig {
cfg := &config.AuthConfig{}
cfg.SetActiveToken("token", "stub")
return cfg
},
}, nil
},
RequestPath: s.URL,
}

require.NoError(t, apiRun(&options))
assert.Contains(t, receivedUA, "GitHub CLI 1.2.3")
assert.Contains(t, receivedUA, "Agent/copilot-cli")
}

func Test_openUserFile(t *testing.T) {
f, err := os.CreateTemp(t.TempDir(), "gh-test")
if err != nil {
Expand Down
1 change: 1 addition & 0 deletions pkg/cmd/factory/default.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ var ssoURLRE = regexp.MustCompile(`\burl=([^;]+)`)
func New(appVersion string, invokingAgent string) *cmdutil.Factory {
f := &cmdutil.Factory{
AppVersion: appVersion,
InvokingAgent: invokingAgent,
Config: configFunc(), // No factory dependencies
ExecutableName: "gh",
}
Expand Down
1 change: 1 addition & 0 deletions pkg/cmdutil/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
type Factory struct {
AppVersion string
ExecutableName string
InvokingAgent string

Browser browser.Browser
ExtensionManager extensions.ExtensionManager
Expand Down
Loading