Merge branch 'trunk' into rerun-err-msg

.github/ISSUE_TEMPLATE/bug_report.md

@@ -9,7 +9,11 @@ assignees: ''
 
 ### Describe the bug
 
-A clear and concise description of what the bug is. Include version by typing `gh --version`.
+A clear and concise description of what the bug is. 
+
+### Affected version
+
+Please run `gh version` and paste the output below.
 
 ### Steps to reproduce the behavior
 

go.mod

@@ -19,7 +19,7 @@ require (
 	github.com/creack/pty v1.1.24
 	github.com/digitorus/timestamp v0.0.0-20231217203849-220c5c2851b7
 	github.com/distribution/reference v0.5.0
-	github.com/gabriel-vasile/mimetype v1.4.7
+	github.com/gabriel-vasile/mimetype v1.4.8
 	github.com/gdamore/tcell/v2 v2.5.4
 	github.com/golang/snappy v0.0.4
 	github.com/google/go-cmp v0.6.0
@@ -51,7 +51,7 @@ require (
 	golang.org/x/term v0.27.0
 	golang.org/x/text v0.21.0
 	google.golang.org/grpc v1.64.1
-	google.golang.org/protobuf v1.36.2
+	google.golang.org/protobuf v1.36.3
 	gopkg.in/h2non/gock.v1 v1.1.2
 	gopkg.in/yaml.v3 v3.0.1
 )

go.sum

@@ -149,8 +149,8 @@ github.com/frankban/quicktest v1.14.6 h1:7Xjx+VpznH+oBnejlPUj8oUpdxnVs4f8XU8WnHk
 github.com/frankban/quicktest v1.14.6/go.mod h1:4ptaffx2x8+WTWXmUCuVU6aPUX1/Mz7zb5vbUoiM6w0=
 github.com/fsnotify/fsnotify v1.7.0 h1:8JEhPFa5W2WU7YfeZzPNqzMP6Lwt7L2715Ggo0nosvA=
 github.com/fsnotify/fsnotify v1.7.0/go.mod h1:40Bi/Hjc2AVfZrqy+aj+yEI+/bRxZnMJyTJwOpGvigM=
-github.com/gabriel-vasile/mimetype v1.4.7 h1:SKFKl7kD0RiPdbht0s7hFtjl489WcQ1VyPW8ZzUMYCA=
-github.com/gabriel-vasile/mimetype v1.4.7/go.mod h1:GDlAgAyIRT27BhFl53XNAFtfjzOkLaF35JdEG0P7LtU=
+github.com/gabriel-vasile/mimetype v1.4.8 h1:FfZ3gj38NjllZIeJAmMhr+qKL8Wu+nOoI3GqacKw1NM=
+github.com/gabriel-vasile/mimetype v1.4.8/go.mod h1:ByKUIKGjh1ODkGM1asKUbQZOLGrPjydw3hYPU2YU9t8=
 github.com/gdamore/encoding v1.0.0 h1:+7OoQ1Bc6eTm5niUzBa0Ctsh6JbMW6Ra+YNuAtDBdko=
 github.com/gdamore/encoding v1.0.0/go.mod h1:alR0ol34c49FCSBLjhosxzcPHQbf2trDkoo5dl+VrEg=
 github.com/gdamore/tcell/v2 v2.5.4 h1:TGU4tSjD3sCL788vFNeJnTdzpNKIw1H5dgLnJRQVv/k=
@@ -548,8 +548,8 @@ google.golang.org/genproto/googleapis/rpc v0.0.0-20240520151616-dc85e6b867a5 h1:
 google.golang.org/genproto/googleapis/rpc v0.0.0-20240520151616-dc85e6b867a5/go.mod h1:EfXuqaE1J41VCDicxHzUDm+8rk+7ZdXzHV0IhO/I6s0=
 google.golang.org/grpc v1.64.1 h1:LKtvyfbX3UGVPFcGqJ9ItpVWW6oN/2XqTxfAnwRRXiA=
 google.golang.org/grpc v1.64.1/go.mod h1:hiQF4LFZelK2WKaP6W0L92zGHtiQdZxk8CrSdvyjeP0=
-google.golang.org/protobuf v1.36.2 h1:R8FeyR1/eLmkutZOM5CWghmo5itiG9z0ktFlTVLuTmU=
-google.golang.org/protobuf v1.36.2/go.mod h1:9fA7Ob0pmnwhb644+1+CVWFRbNajQ6iRojtC/QF5bRE=
+google.golang.org/protobuf v1.36.3 h1:82DV7MYdb8anAVi3qge1wSnMDrnKK7ebr+I0hHRN1BU=
+google.golang.org/protobuf v1.36.3/go.mod h1:9fA7Ob0pmnwhb644+1+CVWFRbNajQ6iRojtC/QF5bRE=
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
 gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c h1:Hei/4ADfdWqJk1ZMxUNpqntNwaWcugrBjAiHlqqRiVk=
 gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c/go.mod h1:JHkPIbrfpd72SG/EVd6muEfDQjcINNoR0C8j2r3qZ4Q=

internal/codespaces/api/api.go

@@ -240,6 +240,8 @@ const (
 	CodespaceStateAvailable = "Available"
 	// CodespaceStateShutdown is the state for a shutdown codespace environment.
 	CodespaceStateShutdown = "Shutdown"
+	// CodespaceStateShuttingDown is the state for a shutting down codespace environment.
+	CodespaceStateShuttingDown = "ShuttingDown"
 	// CodespaceStateStarting is the state for a starting codespace environment.
 	CodespaceStateStarting = "Starting"
 	// CodespaceStateRebuilding is the state for a rebuilding codespace environment.

internal/codespaces/codespaces.go

@@ -13,6 +13,15 @@ import (
 	"github.com/cli/cli/v2/internal/codespaces/connection"
 )
 
+// codespaceStatePollingBackoff is the delay between state polls while waiting for codespaces to become
+// available. It's only exposed so that it can be shortened for testing, otherwise it should not be changed
+var codespaceStatePollingBackoff backoff.BackOff = backoff.NewExponentialBackOff(
+	backoff.WithInitialInterval(1*time.Second),
+	backoff.WithMultiplier(1.02),
+	backoff.WithMaxInterval(10*time.Second),
+	backoff.WithMaxElapsedTime(5*time.Minute),
+)
+
 func connectionReady(codespace *api.Codespace) bool {
 	// If the codespace is not available, it is not ready
 	if codespace.State != api.CodespaceStateAvailable {
@@ -67,41 +76,53 @@ func GetCodespaceConnection(ctx context.Context, progress progressIndicator, api
 
 // waitUntilCodespaceConnectionReady waits for a Codespace to be running and is able to be connected to.
 func waitUntilCodespaceConnectionReady(ctx context.Context, progress progressIndicator, apiClient apiClient, codespace *api.Codespace) (*api.Codespace, error) {
-	if codespace.State != api.CodespaceStateAvailable {
-		progress.StartProgressIndicatorWithLabel("Starting codespace")
-		defer progress.StopProgressIndicator()
-		if err := apiClient.StartCodespace(ctx, codespace.Name); err != nil {
-			return nil, fmt.Errorf("error starting codespace: %w", err)
-		}
+	if connectionReady(codespace) {
+		return codespace, nil
 	}
 
-	if !connectionReady(codespace) {
-		expBackoff := backoff.NewExponentialBackOff()
-		expBackoff.Multiplier = 1.1
-		expBackoff.MaxInterval = 10 * time.Second
-		expBackoff.MaxElapsedTime = 5 * time.Minute
+	progress.StartProgressIndicatorWithLabel("Waiting for codespace to become ready")
+	defer progress.StopProgressIndicator()
 
-		err := backoff.Retry(func() error {
-			var err error
+	lastState := ""
+	firstRetry := true
+
+	err := backoff.Retry(func() error {
+		var err error
+		if firstRetry {
+			firstRetry = false
+		} else {
 			codespace, err = apiClient.GetCodespace(ctx, codespace.Name, true)
 			if err != nil {
 				return backoff.Permanent(fmt.Errorf("error getting codespace: %w", err))
 			}
-
-			if connectionReady(codespace) {
-				return nil
-			}
-
-			return &TimeoutError{message: "codespace not ready yet"}
-		}, backoff.WithContext(expBackoff, ctx))
-		if err != nil {
-			var timeoutErr *TimeoutError
-			if errors.As(err, &timeoutErr) {
-				return nil, errors.New("timed out while waiting for the codespace to start")
-			}
-
-			return nil, err
 		}
+
+		if connectionReady(codespace) {
+			return nil
+		}
+
+		// Only react to changes in the state (so that we don't try to start the codespace twice)
+		if codespace.State != lastState {
+			if codespace.State == api.CodespaceStateShutdown {
+				err = apiClient.StartCodespace(ctx, codespace.Name)
+				if err != nil {
+					return backoff.Permanent(fmt.Errorf("error starting codespace: %w", err))
+				}
+			}
+		}
+
+		lastState = codespace.State
+
+		return &TimeoutError{message: "codespace not ready yet"}
+	}, backoff.WithContext(codespaceStatePollingBackoff, ctx))
+
+	if err != nil {
+		var timeoutErr *TimeoutError
+		if errors.As(err, &timeoutErr) {
+			return nil, errors.New("timed out while waiting for the codespace to start")
+		}
+
+		return nil, err
 	}
 
 	return codespace, nil

internal/codespaces/codespaces_test.go

@@ -0,0 +1,212 @@
+package codespaces
+
+import (
+	"context"
+	"net/http"
+	"testing"
+	"time"
+
+	"github.com/cenkalti/backoff/v4"
+	"github.com/cli/cli/v2/internal/codespaces/api"
+)
+
+func init() {
+	// Set the backoff to 0 for testing so that they run quickly
+	codespaceStatePollingBackoff = backoff.NewConstantBackOff(time.Second * 0)
+}
+
+// This is just enough to trick `connectionReady`
+var readyCodespace = &api.Codespace{
+	State: api.CodespaceStateAvailable,
+	Connection: api.CodespaceConnection{
+		TunnelProperties: api.TunnelProperties{
+			ConnectAccessToken:     "test",
+			ManagePortsAccessToken: "test",
+			ServiceUri:             "test",
+			TunnelId:               "test",
+			ClusterId:              "test",
+			Domain:                 "test",
+		},
+	},
+}
+
+func TestWaitUntilCodespaceConnectionReady_WhenAlreadyReady(t *testing.T) {
+	t.Parallel()
+
+	apiClient := &mockApiClient{}
+	result, err := waitUntilCodespaceConnectionReady(context.Background(), &mockProgressIndicator{}, apiClient, readyCodespace)
+	if err != nil {
+		t.Fatalf("Expected nil error, but was %v", err)
+	}
+	if result.State != api.CodespaceStateAvailable {
+		t.Fatalf("Expected final state to be %s, but was %s", api.CodespaceStateAvailable, result.State)
+	}
+}
+
+func TestWaitUntilCodespaceConnectionReady_PollsApi(t *testing.T) {
+	t.Parallel()
+
+	apiClient := &mockApiClient{
+		onGetCodespace: func() (*api.Codespace, error) {
+			return readyCodespace, nil
+		},
+	}
+	result, err := waitUntilCodespaceConnectionReady(context.Background(), &mockProgressIndicator{}, apiClient, &api.Codespace{State: api.CodespaceStateStarting})
+
+	if err != nil {
+		t.Fatalf("Expected nil error, but was %v", err)
+	}
+	if result.State != api.CodespaceStateAvailable {
+		t.Fatalf("Expected final state to be %s, but was %s", api.CodespaceStateAvailable, result.State)
+	}
+}
+
+func TestWaitUntilCodespaceConnectionReady_StartsCodespace(t *testing.T) {
+	t.Parallel()
+
+	codespace := &api.Codespace{State: api.CodespaceStateShutdown}
+
+	apiClient := &mockApiClient{
+		onGetCodespace: func() (*api.Codespace, error) {
+			return codespace, nil
+		},
+		onStartCodespace: func() error {
+			*codespace = *readyCodespace
+			return nil
+		},
+	}
+	result, err := waitUntilCodespaceConnectionReady(context.Background(), &mockProgressIndicator{}, apiClient, codespace)
+	if err != nil {
+		t.Fatalf("Expected nil error, but was %v", err)
+	}
+	if result.State != api.CodespaceStateAvailable {
+		t.Fatalf("Expected final state to be %s, but was %s", api.CodespaceStateAvailable, result.State)
+	}
+}
+
+func TestWaitUntilCodespaceConnectionReady_PollsCodespaceUntilReady(t *testing.T) {
+	t.Parallel()
+
+	codespace := &api.Codespace{State: api.CodespaceStateShutdown}
+	hasPolled := false
+
+	apiClient := &mockApiClient{
+		onGetCodespace: func() (*api.Codespace, error) {
+			if hasPolled {
+				*codespace = *readyCodespace
+			}
+
+			hasPolled = true
+
+			return codespace, nil
+		},
+		onStartCodespace: func() error {
+			codespace.State = api.CodespaceStateStarting
+			return nil
+		},
+	}
+	result, err := waitUntilCodespaceConnectionReady(context.Background(), &mockProgressIndicator{}, apiClient, codespace)
+	if err != nil {
+		t.Fatalf("Expected nil error, but was %v", err)
+	}
+	if result.State != api.CodespaceStateAvailable {
+		t.Fatalf("Expected final state to be %s, but was %s", api.CodespaceStateAvailable, result.State)
+	}
+}
+
+func TestWaitUntilCodespaceConnectionReady_WaitsForShutdownBeforeStarting(t *testing.T) {
+	t.Parallel()
+
+	codespace := &api.Codespace{State: api.CodespaceStateShuttingDown}
+
+	apiClient := &mockApiClient{
+		onGetCodespace: func() (*api.Codespace, error) {
+			// Make sure that we poll at least once before going to shutdown
+			if codespace.State == api.CodespaceStateShuttingDown {
+				codespace.State = api.CodespaceStateShutdown
+			}
+			return codespace, nil
+		},
+		onStartCodespace: func() error {
+			if codespace.State != api.CodespaceStateShutdown {
+				t.Fatalf("Codespace started from non-shutdown state: %s", codespace.State)
+			}
+			*codespace = *readyCodespace
+			return nil
+		},
+	}
+	result, err := waitUntilCodespaceConnectionReady(context.Background(), &mockProgressIndicator{}, apiClient, codespace)
+	if err != nil {
+		t.Fatalf("Expected nil error, but was %v", err)
+	}
+	if result.State != api.CodespaceStateAvailable {
+		t.Fatalf("Expected final state to be %s, but was %s", api.CodespaceStateAvailable, result.State)
+	}
+}
+
+func TestUntilCodespaceConnectionReady_DoesntStartTwice(t *testing.T) {
+	t.Parallel()
+
+	codespace := &api.Codespace{State: api.CodespaceStateShutdown}
+	didStart := false
+	didPollAfterStart := false
+
+	apiClient := &mockApiClient{
+		onGetCodespace: func() (*api.Codespace, error) {
+			// Make sure that we are in shutdown state for one poll after starting to make sure we don't try to start again
+			if didPollAfterStart {
+				*codespace = *readyCodespace
+			}
+
+			if didStart {
+				didPollAfterStart = true
+			}
+
+			return codespace, nil
+		},
+		onStartCodespace: func() error {
+			if didStart {
+				t.Fatal("Should not start multiple times")
+			}
+			didStart = true
+			return nil
+		},
+	}
+	result, err := waitUntilCodespaceConnectionReady(context.Background(), &mockProgressIndicator{}, apiClient, codespace)
+	if err != nil {
+		t.Fatalf("Expected nil error, but was %v", err)
+	}
+	if result.State != api.CodespaceStateAvailable {
+		t.Fatalf("Expected final state to be %s, but was %s", api.CodespaceStateAvailable, result.State)
+	}
+}
+
+type mockApiClient struct {
+	onStartCodespace func() error
+	onGetCodespace   func() (*api.Codespace, error)
+}
+
+func (m *mockApiClient) StartCodespace(ctx context.Context, name string) error {
+	if m.onStartCodespace == nil {
+		panic("onStartCodespace not set and StartCodespace was called")
+	}
+
+	return m.onStartCodespace()
+}
+
+func (m *mockApiClient) GetCodespace(ctx context.Context, name string, includeConnection bool) (*api.Codespace, error) {
+	if m.onGetCodespace == nil {
+		panic("onGetCodespace not set and GetCodespace was called")
+	}
+
+	return m.onGetCodespace()
+}
+
+func (m *mockApiClient) HTTPClient() (*http.Client, error) {
+	panic("Not implemented")
+}
+
+type mockProgressIndicator struct{}
+
+func (m *mockProgressIndicator) StartProgressIndicatorWithLabel(s string) {}
+func (m *mockProgressIndicator) StopProgressIndicator()                   {}

pkg/cmd/gist/edit/edit.go

@@ -108,6 +108,10 @@ func editRun(opts *EditOptions) error {
 	if gistID == "" {
 		cs := opts.IO.ColorScheme()
 		if gistID == "" {
+			if !opts.IO.CanPrompt() {
+				return cmdutil.FlagErrorf("gist ID or URL required when not running interactively")
+			}
+
 			gist, err := shared.PromptGists(opts.Prompter, client, host, cs)
 			if err != nil {
 				return err

pkg/cmd/gist/edit/edit_test.go

@@ -3,11 +3,13 @@ package edit
 import (
 	"bytes"
 	"encoding/json"
+	"fmt"
 	"io"
 	"net/http"
 	"os"
 	"path/filepath"
 	"testing"
+	"time"
 
 	"github.com/cli/cli/v2/internal/config"
 	"github.com/cli/cli/v2/internal/gh"
@@ -141,23 +143,31 @@ func Test_editRun(t *testing.T) {
 	require.NoError(t, err)
 
 	tests := []struct {
-		name          string
-		opts          *EditOptions
-		gist          *shared.Gist
-		httpStubs     func(*httpmock.Registry)
-		prompterStubs func(*prompter.MockPrompter)
-		nontty        bool
-		stdin         string
-		wantErr       string
-		wantParams    map[string]interface{}
+		name                      string
+		opts                      *EditOptions
+		mockGist                  *shared.Gist
+		mockGistList              bool
+		httpStubs                 func(*httpmock.Registry)
+		prompterStubs             func(*prompter.MockPrompter)
+		isTTY                     bool
+		stdin                     string
+		wantErr                   string
+		wantLastRequestParameters map[string]interface{}
 	}{
 		{
 			name:    "no such gist",
 			wantErr: "gist not found: 1234",
+			opts: &EditOptions{
+				Selector: "1234",
+			},
 		},
 		{
-			name: "one file",
-			gist: &shared.Gist{
+			name:  "one file",
+			isTTY: false,
+			opts: &EditOptions{
+				Selector: "1234",
+			},
+			mockGist: &shared.Gist{
 				ID: "1234",
 				Files: map[string]*shared.GistFile{
 					"cicada.txt": {
@@ -172,7 +182,7 @@ func Test_editRun(t *testing.T) {
 				reg.Register(httpmock.REST("POST", "gists/1234"),
 					httpmock.StatusStringResponse(201, "{}"))
 			},
-			wantParams: map[string]interface{}{
+			wantLastRequestParameters: map[string]interface{}{
 				"description": "",
 				"files": map[string]interface{}{
 					"cicada.txt": map[string]interface{}{
@@ -183,7 +193,9 @@ func Test_editRun(t *testing.T) {
 			},
 		},
 		{
-			name: "multiple files, submit",
+			name:         "multiple files, submit, with TTY",
+			isTTY:        true,
+			mockGistList: true,
 			prompterStubs: func(pm *prompter.MockPrompter) {
 				pm.RegisterSelect("Edit which file?",
 					[]string{"cicada.txt", "unix.md"},
@@ -196,7 +208,7 @@ func Test_editRun(t *testing.T) {
 						return prompter.IndexFor(opts, "Submit")
 					})
 			},
-			gist: &shared.Gist{
+			mockGist: &shared.Gist{
 				ID:          "1234",
 				Description: "catbug",
 				Files: map[string]*shared.GistFile{
@@ -215,7 +227,7 @@ func Test_editRun(t *testing.T) {
 				reg.Register(httpmock.REST("POST", "gists/1234"),
 					httpmock.StatusStringResponse(201, "{}"))
 			},
-			wantParams: map[string]interface{}{
+			wantLastRequestParameters: map[string]interface{}{
 				"description": "catbug",
 				"files": map[string]interface{}{
 					"cicada.txt": map[string]interface{}{
@@ -230,7 +242,11 @@ func Test_editRun(t *testing.T) {
 			},
 		},
 		{
-			name: "multiple files, cancel",
+			name:  "multiple files, cancel, with TTY",
+			isTTY: true,
+			opts: &EditOptions{
+				Selector: "1234",
+			},
 			prompterStubs: func(pm *prompter.MockPrompter) {
 				pm.RegisterSelect("Edit which file?",
 					[]string{"cicada.txt", "unix.md"},
@@ -244,7 +260,7 @@ func Test_editRun(t *testing.T) {
 					})
 			},
 			wantErr: "CancelError",
-			gist: &shared.Gist{
+			mockGist: &shared.Gist{
 				ID: "1234",
 				Files: map[string]*shared.GistFile{
 					"cicada.txt": {
@@ -263,7 +279,10 @@ func Test_editRun(t *testing.T) {
 		},
 		{
 			name: "not change",
-			gist: &shared.Gist{
+			opts: &EditOptions{
+				Selector: "1234",
+			},
+			mockGist: &shared.Gist{
 				ID: "1234",
 				Files: map[string]*shared.GistFile{
 					"cicada.txt": {
@@ -277,7 +296,10 @@ func Test_editRun(t *testing.T) {
 		},
 		{
 			name: "another user's gist",
-			gist: &shared.Gist{
+			opts: &EditOptions{
+				Selector: "1234",
+			},
+			mockGist: &shared.Gist{
 				ID: "1234",
 				Files: map[string]*shared.GistFile{
 					"cicada.txt": {
@@ -292,7 +314,11 @@ func Test_editRun(t *testing.T) {
 		},
 		{
 			name: "add file to existing gist",
-			gist: &shared.Gist{
+			opts: &EditOptions{
+				AddFilename: fileToAdd,
+				Selector:    "1234",
+			},
+			mockGist: &shared.Gist{
 				ID: "1234",
 				Files: map[string]*shared.GistFile{
 					"sample.txt": {
@@ -307,16 +333,14 @@ func Test_editRun(t *testing.T) {
 				reg.Register(httpmock.REST("POST", "gists/1234"),
 					httpmock.StatusStringResponse(201, "{}"))
 			},
-			opts: &EditOptions{
-				AddFilename: fileToAdd,
-			},
 		},
 		{
 			name: "change description",
 			opts: &EditOptions{
 				Description: "my new description",
+				Selector:    "1234",
 			},
-			gist: &shared.Gist{
+			mockGist: &shared.Gist{
 				ID:          "1234",
 				Description: "my old description",
 				Files: map[string]*shared.GistFile{
@@ -331,7 +355,7 @@ func Test_editRun(t *testing.T) {
 				reg.Register(httpmock.REST("POST", "gists/1234"),
 					httpmock.StatusStringResponse(201, "{}"))
 			},
-			wantParams: map[string]interface{}{
+			wantLastRequestParameters: map[string]interface{}{
 				"description": "my new description",
 				"files": map[string]interface{}{
 					"sample.txt": map[string]interface{}{
@@ -343,7 +367,12 @@ func Test_editRun(t *testing.T) {
 		},
 		{
 			name: "add file to existing gist from source parameter",
-			gist: &shared.Gist{
+			opts: &EditOptions{
+				AddFilename: "from_source.txt",
+				SourceFile:  fileToAdd,
+				Selector:    "1234",
+			},
+			mockGist: &shared.Gist{
 				ID: "1234",
 				Files: map[string]*shared.GistFile{
 					"sample.txt": {
@@ -358,11 +387,7 @@ func Test_editRun(t *testing.T) {
 				reg.Register(httpmock.REST("POST", "gists/1234"),
 					httpmock.StatusStringResponse(201, "{}"))
 			},
-			opts: &EditOptions{
-				AddFilename: "from_source.txt",
-				SourceFile:  fileToAdd,
-			},
-			wantParams: map[string]interface{}{
+			wantLastRequestParameters: map[string]interface{}{
 				"description": "",
 				"files": map[string]interface{}{
 					"from_source.txt": map[string]interface{}{
@@ -374,7 +399,12 @@ func Test_editRun(t *testing.T) {
 		},
 		{
 			name: "add file to existing gist from stdin",
-			gist: &shared.Gist{
+			opts: &EditOptions{
+				AddFilename: "from_source.txt",
+				SourceFile:  "-",
+				Selector:    "1234",
+			},
+			mockGist: &shared.Gist{
 				ID: "1234",
 				Files: map[string]*shared.GistFile{
 					"sample.txt": {
@@ -389,12 +419,8 @@ func Test_editRun(t *testing.T) {
 				reg.Register(httpmock.REST("POST", "gists/1234"),
 					httpmock.StatusStringResponse(201, "{}"))
 			},
-			opts: &EditOptions{
-				AddFilename: "from_source.txt",
-				SourceFile:  "-",
-			},
 			stdin: "data from stdin",
-			wantParams: map[string]interface{}{
+			wantLastRequestParameters: map[string]interface{}{
 				"description": "",
 				"files": map[string]interface{}{
 					"from_source.txt": map[string]interface{}{
@@ -406,7 +432,11 @@ func Test_editRun(t *testing.T) {
 		},
 		{
 			name: "remove file, file does not exist",
-			gist: &shared.Gist{
+			opts: &EditOptions{
+				RemoveFilename: "sample2.txt",
+				Selector:       "1234",
+			},
+			mockGist: &shared.Gist{
 				ID: "1234",
 				Files: map[string]*shared.GistFile{
 					"sample.txt": {
@@ -417,14 +447,15 @@ func Test_editRun(t *testing.T) {
 				},
 				Owner: &shared.GistOwner{Login: "octocat"},
 			},
-			opts: &EditOptions{
-				RemoveFilename: "sample2.txt",
-			},
 			wantErr: "gist has no file \"sample2.txt\"",
 		},
 		{
 			name: "remove file from existing gist",
-			gist: &shared.Gist{
+			opts: &EditOptions{
+				RemoveFilename: "sample2.txt",
+				Selector:       "1234",
+			},
+			mockGist: &shared.Gist{
 				ID: "1234",
 				Files: map[string]*shared.GistFile{
 					"sample.txt": {
@@ -444,10 +475,7 @@ func Test_editRun(t *testing.T) {
 				reg.Register(httpmock.REST("POST", "gists/1234"),
 					httpmock.StatusStringResponse(201, "{}"))
 			},
-			opts: &EditOptions{
-				RemoveFilename: "sample2.txt",
-			},
-			wantParams: map[string]interface{}{
+			wantLastRequestParameters: map[string]interface{}{
 				"description": "",
 				"files": map[string]interface{}{
 					"sample.txt": map[string]interface{}{
@@ -460,7 +488,11 @@ func Test_editRun(t *testing.T) {
 		},
 		{
 			name: "edit gist using file from source parameter",
-			gist: &shared.Gist{
+			opts: &EditOptions{
+				SourceFile: fileToAdd,
+				Selector:   "1234",
+			},
+			mockGist: &shared.Gist{
 				ID: "1234",
 				Files: map[string]*shared.GistFile{
 					"sample.txt": {
@@ -475,10 +507,7 @@ func Test_editRun(t *testing.T) {
 				reg.Register(httpmock.REST("POST", "gists/1234"),
 					httpmock.StatusStringResponse(201, "{}"))
 			},
-			opts: &EditOptions{
-				SourceFile: fileToAdd,
-			},
-			wantParams: map[string]interface{}{
+			wantLastRequestParameters: map[string]interface{}{
 				"description": "",
 				"files": map[string]interface{}{
 					"sample.txt": map[string]interface{}{
@@ -490,7 +519,11 @@ func Test_editRun(t *testing.T) {
 		},
 		{
 			name: "edit gist using stdin",
-			gist: &shared.Gist{
+			opts: &EditOptions{
+				SourceFile: "-",
+				Selector:   "1234",
+			},
+			mockGist: &shared.Gist{
 				ID: "1234",
 				Files: map[string]*shared.GistFile{
 					"sample.txt": {
@@ -505,11 +538,8 @@ func Test_editRun(t *testing.T) {
 				reg.Register(httpmock.REST("POST", "gists/1234"),
 					httpmock.StatusStringResponse(201, "{}"))
 			},
-			opts: &EditOptions{
-				SourceFile: "-",
-			},
 			stdin: "data from stdin",
-			wantParams: map[string]interface{}{
+			wantLastRequestParameters: map[string]interface{}{
 				"description": "",
 				"files": map[string]interface{}{
 					"sample.txt": map[string]interface{}{
@@ -519,28 +549,74 @@ func Test_editRun(t *testing.T) {
 				},
 			},
 		},
+		{
+			name:  "no arguments notty",
+			isTTY: false,
+			opts: &EditOptions{
+				Selector: "",
+			},
+			wantErr: "gist ID or URL required when not running interactively",
+		},
 	}
 
 	for _, tt := range tests {
 		reg := &httpmock.Registry{}
-		if tt.gist == nil {
+		pm := prompter.NewMockPrompter(t)
+
+		if tt.opts == nil {
+			tt.opts = &EditOptions{}
+		}
+
+		if tt.opts.Selector != "" {
+			// Only register the HTTP stubs for a direct gist lookup if a selector is provided.
+			if tt.mockGist == nil {
+				// If no gist is provided, we expect a 404.
+				reg.Register(httpmock.REST("GET", fmt.Sprintf("gists/%s", tt.opts.Selector)),
+					httpmock.StatusStringResponse(404, "Not Found"))
+			} else {
+				// If a gist is provided, we expect the gist to be fetched.
+				reg.Register(httpmock.REST("GET", fmt.Sprintf("gists/%s", tt.opts.Selector)),
+					httpmock.JSONResponse(tt.mockGist))
+				reg.Register(httpmock.GraphQL(`query UserCurrent\b`),
+					httpmock.StringResponse(`{"data":{"viewer":{"login":"octocat"}}}`))
+			}
+		}
+
+		if tt.mockGistList {
+			sixHours, _ := time.ParseDuration("6h")
+			sixHoursAgo := time.Now().Add(-sixHours)
+			reg.Register(httpmock.GraphQL(`query GistList\b`),
+				httpmock.StringResponse(
+					fmt.Sprintf(`{ "data": { "viewer": { "gists": { "nodes": [
+							{
+								"description": "whatever",
+								"files": [{ "name": "cicada.txt" }, { "name": "unix.md"	}],
+								"isPublic": true,
+								"name": "1234",
+								"updatedAt": "%s"
+							}
+							],
+							"pageInfo": {
+								"hasNextPage": false,
+								"endCursor": "somevaluedoesnotmatter"
+							} }	} }	}`, sixHoursAgo.Format(time.RFC3339))))
 			reg.Register(httpmock.REST("GET", "gists/1234"),
-				httpmock.StatusStringResponse(404, "Not Found"))
-		} else {
-			reg.Register(httpmock.REST("GET", "gists/1234"),
-				httpmock.JSONResponse(tt.gist))
+				httpmock.JSONResponse(tt.mockGist))
 			reg.Register(httpmock.GraphQL(`query UserCurrent\b`),
 				httpmock.StringResponse(`{"data":{"viewer":{"login":"octocat"}}}`))
+
+			gistList := "cicada.txt whatever about 6 hours ago"
+			pm.RegisterSelect("Select a gist",
+				[]string{gistList},
+				func(_, _ string, opts []string) (int, error) {
+					return prompter.IndexFor(opts, gistList)
+				})
 		}
 
 		if tt.httpStubs != nil {
 			tt.httpStubs(reg)
 		}
 
-		if tt.opts == nil {
-			tt.opts = &EditOptions{}
-		}
-
 		tt.opts.Edit = func(_, _, _ string, _ *iostreams.IOStreams) (string, error) {
 			return "new file content", nil
 		}
@@ -550,17 +626,17 @@ func Test_editRun(t *testing.T) {
 		}
 		ios, stdin, stdout, stderr := iostreams.Test()
 		stdin.WriteString(tt.stdin)
-		ios.SetStdoutTTY(!tt.nontty)
-		ios.SetStdinTTY(!tt.nontty)
+		ios.SetStdoutTTY(tt.isTTY)
+		ios.SetStdinTTY(tt.isTTY)
+		ios.SetStderrTTY(tt.isTTY)
+
 		tt.opts.IO = ios
-		tt.opts.Selector = "1234"
 
 		tt.opts.Config = func() (gh.Config, error) {
 			return config.NewBlankConfig(), nil
 		}
 
 		t.Run(tt.name, func(t *testing.T) {
-			pm := prompter.NewMockPrompter(t)
 			if tt.prompterStubs != nil {
 				tt.prompterStubs(pm)
 			}
@@ -574,14 +650,21 @@ func Test_editRun(t *testing.T) {
 			}
 			assert.NoError(t, err)
 
-			if tt.wantParams != nil {
-				bodyBytes, _ := io.ReadAll(reg.Requests[2].Body)
+			if tt.wantLastRequestParameters != nil {
+				// Currently only checking that the last request has
+				// the expected request parameters.
+				//
+				// This might need to be changed, if a test were to be added
+				// that needed to check that a request other than the last
+				// has the desired parameters.
+				lastRequest := reg.Requests[len(reg.Requests)-1]
+				bodyBytes, _ := io.ReadAll(lastRequest.Body)
 				reqBody := make(map[string]interface{})
 				err = json.Unmarshal(bodyBytes, &reqBody)
 				if err != nil {
 					t.Fatalf("error decoding JSON: %v", err)
 				}
-				assert.Equal(t, tt.wantParams, reqBody)
+				assert.Equal(t, tt.wantLastRequestParameters, reqBody)
 			}
 
 			assert.Equal(t, "", stdout.String())

pkg/cmd/gist/view/view.go

@@ -89,6 +89,10 @@ func viewRun(opts *ViewOptions) error {
 
 	cs := opts.IO.ColorScheme()
 	if gistID == "" {
+		if !opts.IO.CanPrompt() {
+			return cmdutil.FlagErrorf("gist ID or URL required when not running interactively")
+		}
+
 		gist, err := shared.PromptGists(opts.Prompter, client, hostname, cs)
 		if err != nil {
 			return err

pkg/cmd/gist/view/view_test.go

@@ -16,6 +16,7 @@ import (
 	"github.com/cli/cli/v2/pkg/iostreams"
 	"github.com/google/shlex"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestNewCmdView(t *testing.T) {
@@ -94,6 +95,7 @@ func TestNewCmdView(t *testing.T) {
 				gotOpts = opts
 				return nil
 			})
+
 			cmd.SetArgs(argv)
 			cmd.SetIn(&bytes.Buffer{})
 			cmd.SetOut(&bytes.Buffer{})
@@ -114,25 +116,28 @@ func Test_viewRun(t *testing.T) {
 		name         string
 		opts         *ViewOptions
 		wantOut      string
-		gist         *shared.Gist
-		wantErr      bool
+		mockGist     *shared.Gist
 		mockGistList bool
+		isTTY        bool
+		wantErr      string
 	}{
 		{
-			name: "no such gist",
+			name:  "no such gist",
+			isTTY: false,
 			opts: &ViewOptions{
 				Selector:  "1234",
 				ListFiles: false,
 			},
-			wantErr: true,
+			wantErr: "not found",
 		},
 		{
-			name: "one file",
+			name:  "one file",
+			isTTY: true,
 			opts: &ViewOptions{
 				Selector:  "1234",
 				ListFiles: false,
 			},
-			gist: &shared.Gist{
+			mockGist: &shared.Gist{
 				Files: map[string]*shared.GistFile{
 					"cicada.txt": {
 						Content: "bwhiizzzbwhuiiizzzz",
@@ -143,13 +148,14 @@ func Test_viewRun(t *testing.T) {
 			wantOut: "bwhiizzzbwhuiiizzzz\n",
 		},
 		{
-			name: "one file, no ID supplied",
+			name:  "one file, no ID supplied",
+			isTTY: true,
 			opts: &ViewOptions{
 				Selector:  "",
 				ListFiles: false,
 			},
 			mockGistList: true,
-			gist: &shared.Gist{
+			mockGist: &shared.Gist{
 				Files: map[string]*shared.GistFile{
 					"cicada.txt": {
 						Content: "test interactive mode",
@@ -160,13 +166,19 @@ func Test_viewRun(t *testing.T) {
 			wantOut: "test interactive mode\n",
 		},
 		{
-			name: "filename selected",
+			name:    "no arguments notty",
+			isTTY:   false,
+			wantErr: "gist ID or URL required when not running interactively",
+		},
+		{
+			name:  "filename selected",
+			isTTY: true,
 			opts: &ViewOptions{
 				Selector:  "1234",
 				Filename:  "cicada.txt",
 				ListFiles: false,
 			},
-			gist: &shared.Gist{
+			mockGist: &shared.Gist{
 				Files: map[string]*shared.GistFile{
 					"cicada.txt": {
 						Content: "bwhiizzzbwhuiiizzzz",
@@ -181,14 +193,15 @@ func Test_viewRun(t *testing.T) {
 			wantOut: "bwhiizzzbwhuiiizzzz\n",
 		},
 		{
-			name: "filename selected, raw",
+			name:  "filename selected, raw",
+			isTTY: true,
 			opts: &ViewOptions{
 				Selector:  "1234",
 				Filename:  "cicada.txt",
 				Raw:       true,
 				ListFiles: false,
 			},
-			gist: &shared.Gist{
+			mockGist: &shared.Gist{
 				Files: map[string]*shared.GistFile{
 					"cicada.txt": {
 						Content: "bwhiizzzbwhuiiizzzz",
@@ -203,12 +216,13 @@ func Test_viewRun(t *testing.T) {
 			wantOut: "bwhiizzzbwhuiiizzzz\n",
 		},
 		{
-			name: "multiple files, no description",
+			name:  "multiple files, no description",
+			isTTY: true,
 			opts: &ViewOptions{
 				Selector:  "1234",
 				ListFiles: false,
 			},
-			gist: &shared.Gist{
+			mockGist: &shared.Gist{
 				Files: map[string]*shared.GistFile{
 					"cicada.txt": {
 						Content: "bwhiizzzbwhuiiizzzz",
@@ -223,12 +237,13 @@ func Test_viewRun(t *testing.T) {
 			wantOut: "cicada.txt\n\nbwhiizzzbwhuiiizzzz\n\nfoo.md\n\n\n  # foo                                                                       \n\n",
 		},
 		{
-			name: "multiple files, trailing newlines",
+			name:  "multiple files, trailing newlines",
+			isTTY: true,
 			opts: &ViewOptions{
 				Selector:  "1234",
 				ListFiles: false,
 			},
-			gist: &shared.Gist{
+			mockGist: &shared.Gist{
 				Files: map[string]*shared.GistFile{
 					"cicada.txt": {
 						Content: "bwhiizzzbwhuiiizzzz\n",
@@ -243,12 +258,13 @@ func Test_viewRun(t *testing.T) {
 			wantOut: "cicada.txt\n\nbwhiizzzbwhuiiizzzz\n\nfoo.txt\n\nbar\n",
 		},
 		{
-			name: "multiple files, description",
+			name:  "multiple files, description",
+			isTTY: true,
 			opts: &ViewOptions{
 				Selector:  "1234",
 				ListFiles: false,
 			},
-			gist: &shared.Gist{
+			mockGist: &shared.Gist{
 				Description: "some files",
 				Files: map[string]*shared.GistFile{
 					"cicada.txt": {
@@ -264,13 +280,14 @@ func Test_viewRun(t *testing.T) {
 			wantOut: "some files\n\ncicada.txt\n\nbwhiizzzbwhuiiizzzz\n\nfoo.md\n\n\n                                                                              \n  • foo                                                                       \n\n",
 		},
 		{
-			name: "multiple files, raw",
+			name:  "multiple files, raw",
+			isTTY: true,
 			opts: &ViewOptions{
 				Selector:  "1234",
 				Raw:       true,
 				ListFiles: false,
 			},
-			gist: &shared.Gist{
+			mockGist: &shared.Gist{
 				Description: "some files",
 				Files: map[string]*shared.GistFile{
 					"cicada.txt": {
@@ -286,13 +303,14 @@ func Test_viewRun(t *testing.T) {
 			wantOut: "some files\n\ncicada.txt\n\nbwhiizzzbwhuiiizzzz\n\nfoo.md\n\n- foo\n",
 		},
 		{
-			name: "one file, list files",
+			name:  "one file, list files",
+			isTTY: true,
 			opts: &ViewOptions{
 				Selector:  "1234",
 				Raw:       false,
 				ListFiles: true,
 			},
-			gist: &shared.Gist{
+			mockGist: &shared.Gist{
 				Description: "some files",
 				Files: map[string]*shared.GistFile{
 					"cicada.txt": {
@@ -304,13 +322,14 @@ func Test_viewRun(t *testing.T) {
 			wantOut: "cicada.txt\n",
 		},
 		{
-			name: "multiple file, list files",
+			name:  "multiple file, list files",
+			isTTY: true,
 			opts: &ViewOptions{
 				Selector:  "1234",
 				Raw:       false,
 				ListFiles: true,
 			},
-			gist: &shared.Gist{
+			mockGist: &shared.Gist{
 				Description: "some files",
 				Files: map[string]*shared.GistFile{
 					"cicada.txt": {
@@ -329,12 +348,12 @@ func Test_viewRun(t *testing.T) {
 
 	for _, tt := range tests {
 		reg := &httpmock.Registry{}
-		if tt.gist == nil {
+		if tt.mockGist == nil {
 			reg.Register(httpmock.REST("GET", "gists/1234"),
 				httpmock.StatusStringResponse(404, "Not Found"))
 		} else {
 			reg.Register(httpmock.REST("GET", "gists/1234"),
-				httpmock.JSONResponse(tt.gist))
+				httpmock.JSONResponse(tt.mockGist))
 		}
 
 		if tt.opts == nil {
@@ -376,16 +395,20 @@ func Test_viewRun(t *testing.T) {
 		}
 
 		ios, _, stdout, _ := iostreams.Test()
-		ios.SetStdoutTTY(true)
+		ios.SetStdoutTTY(tt.isTTY)
+		ios.SetStdinTTY(tt.isTTY)
+		ios.SetStderrTTY(tt.isTTY)
+
 		tt.opts.IO = ios
 
 		t.Run(tt.name, func(t *testing.T) {
 			err := viewRun(tt.opts)
-			if tt.wantErr {
-				assert.Error(t, err)
+			if tt.wantErr != "" {
+				require.EqualError(t, err, tt.wantErr)
 				return
+			} else {
+				require.NoError(t, err)
 			}
-			assert.NoError(t, err)
 
 			assert.Equal(t, tt.wantOut, stdout.String())
 			reg.Verify(t)

pkg/cmd/root/extension.go

@@ -52,20 +52,25 @@ func NewCmdExtension(io *iostreams.IOStreams, em extensions.ExtensionManager, ex
 		},
 		// PostRun handles communicating extension release information if found
 		PostRun: func(c *cobra.Command, args []string) {
-			releaseInfo := <-updateMessageChan
-			if releaseInfo != nil {
-				stderr := io.ErrOut
-				fmt.Fprintf(stderr, "\n\n%s %s → %s\n",
-					cs.Yellowf("A new release of %s is available:", ext.Name()),
-					cs.Cyan(strings.TrimPrefix(ext.CurrentVersion(), "v")),
-					cs.Cyan(strings.TrimPrefix(releaseInfo.Version, "v")))
-				if ext.IsPinned() {
-					fmt.Fprintf(stderr, "To upgrade, run: gh extension upgrade %s --force\n", ext.Name())
-				} else {
-					fmt.Fprintf(stderr, "To upgrade, run: gh extension upgrade %s\n", ext.Name())
+			select {
+			case releaseInfo := <-updateMessageChan:
+				if releaseInfo != nil {
+					stderr := io.ErrOut
+					fmt.Fprintf(stderr, "\n\n%s %s → %s\n",
+						cs.Yellowf("A new release of %s is available:", ext.Name()),
+						cs.Cyan(strings.TrimPrefix(ext.CurrentVersion(), "v")),
+						cs.Cyan(strings.TrimPrefix(releaseInfo.Version, "v")))
+					if ext.IsPinned() {
+						fmt.Fprintf(stderr, "To upgrade, run: gh extension upgrade %s --force\n", ext.Name())
+					} else {
+						fmt.Fprintf(stderr, "To upgrade, run: gh extension upgrade %s\n", ext.Name())
+					}
+					fmt.Fprintf(stderr, "%s\n\n",
+						cs.Yellow(releaseInfo.URL))
 				}
-				fmt.Fprintf(stderr, "%s\n\n",
-					cs.Yellow(releaseInfo.URL))
+			default:
+				// Do not make the user wait for extension update check if incomplete by this time.
+				// This is being handled in non-blocking default as there is no context to cancel like in gh update checks.
 			}
 		},
 		GroupID: "extension",

pkg/cmd/root/extension_test.go

@@ -1,8 +1,10 @@
 package root_test
 
 import (
+	"fmt"
 	"io"
 	"testing"
+	"time"
 
 	"github.com/MakeNowJust/heredoc"
 	"github.com/cli/cli/v2/internal/update"
@@ -121,6 +123,10 @@ func TestNewCmdExtension_Updates(t *testing.T) {
 		em := &extensions.ExtensionManagerMock{
 			DispatchFunc: func(args []string, stdin io.Reader, stdout io.Writer, stderr io.Writer) (bool, error) {
 				// Assume extension executed / dispatched without problems as test is focused on upgrade checking.
+				// Sleep for 100 milliseconds to allow update checking logic to complete. This would be better
+				// served by making the behaviour controllable by channels, but it's a larger change than desired
+				// just to improve the test.
+				time.Sleep(100 * time.Millisecond)
 				return true, nil
 			},
 		}
@@ -169,3 +175,62 @@ func TestNewCmdExtension_Updates(t *testing.T) {
 		}
 	}
 }
+
+func TestNewCmdExtension_UpdateCheckIsNonblocking(t *testing.T) {
+	ios, _, _, _ := iostreams.Test()
+
+	em := &extensions.ExtensionManagerMock{
+		DispatchFunc: func(args []string, stdin io.Reader, stdout io.Writer, stderr io.Writer) (bool, error) {
+			// Assume extension executed / dispatched without problems as test is focused on upgrade checking.
+			return true, nil
+		},
+	}
+
+	ext := &extensions.ExtensionMock{
+		CurrentVersionFunc: func() string {
+			return "1.0.0"
+		},
+		IsPinnedFunc: func() bool {
+			return false
+		},
+		LatestVersionFunc: func() string {
+			return "2.0.0"
+		},
+		NameFunc: func() string {
+			return "major-update"
+		},
+		UpdateAvailableFunc: func() bool {
+			return true
+		},
+		URLFunc: func() string {
+			return "https//github.com/dne/major-update"
+		},
+	}
+
+	// When the extension command is executed, the checkFunc will run in the background longer than the extension dispatch.
+	// If the update check is non-blocking, then the extension command will complete immediately while checkFunc is still running.
+	checkFunc := func(em extensions.ExtensionManager, ext extensions.Extension) (*update.ReleaseInfo, error) {
+		time.Sleep(30 * time.Second)
+		return nil, fmt.Errorf("update check should not have completed")
+	}
+
+	cmd := root.NewCmdExtension(ios, em, ext, checkFunc)
+
+	// The test whether update check is non-blocking is based on how long it takes for the extension command execution.
+	// If there is no wait time as checkFunc is sleeping sufficiently long, we can trust update check is non-blocking.
+	// Otherwise, if any amount of wait is encountered, it is a decent indicator that update checking is blocking.
+	// This is not an ideal test and indicates the update design should be revisited to be easier to understand and manage.
+	completed := make(chan struct{})
+	go func() {
+		_, err := cmd.ExecuteC()
+		require.NoError(t, err)
+		close(completed)
+	}()
+
+	select {
+	case <-completed:
+		// Expected behavior assuming extension dispatch exits immediately while checkFunc is still running.
+	case <-time.After(1 * time.Second):
+		t.Fatal("extension update check should have exited")
+	}
+}