diff --git a/AGENTS.md b/AGENTS.md index 98024f533c3..b04e6b77557 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -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()`. diff --git a/internal/authflow/flow.go b/internal/authflow/flow.go index ebb5b37a037..0a195168f26 100644 --- a/internal/authflow/flow.go +++ b/internal/authflow/flow.go @@ -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 } @@ -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 { diff --git a/internal/authflow/flow_test.go b/internal/authflow/flow_test.go index b7ba1f64a78..68ddaeb905e 100644 --- a/internal/authflow/flow_test.go +++ b/internal/authflow/flow_test.go @@ -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 diff --git a/pkg/cmd/api/api.go b/pkg/cmd/api/api.go index 8e4b2edd45d..fb641457f0f 100644 --- a/pkg/cmd/api/api.go +++ b/pkg/cmd/api/api.go @@ -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 @@ -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{ @@ -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, diff --git a/pkg/cmd/api/api_test.go b/pkg/cmd/api/api_test.go index bf4d51c4c0d..c242e1ad5b9 100644 --- a/pkg/cmd/api/api_test.go +++ b/pkg/cmd/api/api_test.go @@ -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 { diff --git a/pkg/cmd/factory/default.go b/pkg/cmd/factory/default.go index cdbed20af40..7afc6baa758 100644 --- a/pkg/cmd/factory/default.go +++ b/pkg/cmd/factory/default.go @@ -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", } diff --git a/pkg/cmdutil/factory.go b/pkg/cmdutil/factory.go index b90960b3140..f746ec8978d 100644 --- a/pkg/cmdutil/factory.go +++ b/pkg/cmdutil/factory.go @@ -19,6 +19,7 @@ import ( type Factory struct { AppVersion string ExecutableName string + InvokingAgent string Browser browser.Browser ExtensionManager extensions.ExtensionManager