From a2d75d12f67ec4c83462f50bc0152f3269e86066 Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Fri, 29 Aug 2025 18:21:47 -0600 Subject: [PATCH 1/9] Add --web flag to agent-task list command Introduces a --web flag to the agent-task list command, allowing users to open the agent tasks page in their browser. Updates tests to cover the new flag and browser interaction. --- pkg/cmd/agent-task/list/list.go | 20 +++++++-- pkg/cmd/agent-task/list/list_test.go | 67 ++++++++++++++++++++++------ 2 files changed, 71 insertions(+), 16 deletions(-) diff --git a/pkg/cmd/agent-task/list/list.go b/pkg/cmd/agent-task/list/list.go index 43f409ffe..d2e717b72 100644 --- a/pkg/cmd/agent-task/list/list.go +++ b/pkg/cmd/agent-task/list/list.go @@ -5,9 +5,11 @@ import ( "fmt" "time" + "github.com/cli/cli/v2/internal/browser" "github.com/cli/cli/v2/internal/gh" "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/internal/tableprinter" + "github.com/cli/cli/v2/internal/text" "github.com/cli/cli/v2/pkg/cmd/agent-task/capi" "github.com/cli/cli/v2/pkg/cmd/agent-task/shared" prShared "github.com/cli/cli/v2/pkg/cmd/pr/shared" @@ -25,14 +27,17 @@ type ListOptions struct { Limit int CapiClient func() (*capi.CAPIClient, error) BaseRepo func() (ghrepo.Interface, error) + Web bool + Browser browser.Browser } // NewCmdList creates the list command func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Command { opts := &ListOptions{ - IO: f.IOStreams, - Config: f.Config, - Limit: defaultLimit, + IO: f.IOStreams, + Config: f.Config, + Limit: defaultLimit, + Browser: f.Browser, } cmd := &cobra.Command{ @@ -59,6 +64,7 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman } cmd.Flags().IntVarP(&opts.Limit, "limit", "L", defaultLimit, fmt.Sprintf("Maximum number of agent tasks to fetch (default %d)", defaultLimit)) + cmd.Flags().BoolVarP(&opts.Web, "web", "w", false, "Open agent tasks in the browser") opts.CapiClient = func() (*capi.CAPIClient, error) { cfg, err := opts.Config() @@ -77,6 +83,14 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman } func listRun(opts *ListOptions) error { + if opts.Web { + const webURL = "https://github.com/copilot/agents" + if opts.IO.IsStdoutTTY() { + fmt.Fprintf(opts.IO.ErrOut, "Opening %s in your browser.\n", text.DisplayURL(webURL)) + } + return opts.Browser.Browse(webURL) + } + if opts.Limit <= 0 { opts.Limit = defaultLimit } diff --git a/pkg/cmd/agent-task/list/list_test.go b/pkg/cmd/agent-task/list/list_test.go index f05d61b9e..fc45b34d2 100644 --- a/pkg/cmd/agent-task/list/list_test.go +++ b/pkg/cmd/agent-task/list/list_test.go @@ -8,6 +8,7 @@ import ( "time" "github.com/MakeNowJust/heredoc" + "github.com/cli/cli/v2/internal/browser" "github.com/cli/cli/v2/internal/config" "github.com/cli/cli/v2/internal/gh" "github.com/cli/cli/v2/internal/ghrepo" @@ -44,6 +45,14 @@ func TestNewCmdList(t *testing.T) { args: "--limit 0", wantErr: "invalid limit: 0", }, + { + name: "web flag", + args: "--web", + wantOpts: ListOptions{ + Limit: defaultLimit, + Web: true, + }, + }, } for _, tt := range tests { @@ -62,6 +71,7 @@ func TestNewCmdList(t *testing.T) { } require.NoError(t, err) assert.Equal(t, tt.wantOpts.Limit, gotOpts.Limit) + assert.Equal(t, tt.wantOpts.Web, gotOpts.Web) }) } } @@ -72,14 +82,17 @@ func Test_listRun(t *testing.T) { createdAt := sixHoursAgo.Format(time.RFC3339) tests := []struct { - name string - tty bool - stubs func(*httpmock.Registry) - baseRepo ghrepo.Interface - baseRepoErr error - limit int - wantOut string - wantErr error + name string + tty bool + stubs func(*httpmock.Registry) + baseRepo ghrepo.Interface + baseRepoErr error + limit int + web bool + wantOut string + wantErr error + wantStderr string + wantBrowserURL string }{ { name: "no sessions", @@ -166,6 +179,14 @@ func Test_listRun(t *testing.T) { r6 #306 OWNER/REPO mystery about 6 hours ago `), }, + { + name: "web mode", + tty: true, + web: true, + wantOut: "", + wantStderr: "Opening https://github.com/copilot/agents in your browser.\n", + wantBrowserURL: "https://github.com/copilot/agents", + }, } for _, tt := range tests { @@ -179,16 +200,28 @@ func Test_listRun(t *testing.T) { cfg.Set("github.com", "oauth_token", "OTOKEN") authCfg := cfg.Authentication() - ios, _, stdout, _ := iostreams.Test() + ios, _, stdout, stderr := iostreams.Test() ios.SetStdoutTTY(tt.tty) + var br *browser.Stub + if tt.web { + br = &browser.Stub{} + } + httpClient := &http.Client{Transport: reg} capiClient := capi.NewCAPIClient(httpClient, authCfg) opts := &ListOptions{ - IO: ios, - Config: func() (gh.Config, error) { return cfg, nil }, - Limit: tt.limit, - CapiClient: func() (*capi.CAPIClient, error) { return capiClient, nil }, + IO: ios, + Config: func() (gh.Config, error) { return cfg, nil }, + Limit: tt.limit, + Web: tt.web, + Browser: br, + CapiClient: func() (*capi.CAPIClient, error) { + if tt.web { + panic("CapiClient should not be invoked when --web is set") + } + return capiClient, nil + }, } if tt.baseRepo != nil || tt.baseRepoErr != nil { baseRepo := tt.baseRepo @@ -205,6 +238,14 @@ func Test_listRun(t *testing.T) { } got := stdout.String() require.Equal(t, tt.wantOut, got) + if tt.wantStderr != "" { + require.Equal(t, tt.wantStderr, stderr.String()) + } else { + require.Equal(t, "", stderr.String()) + } + if tt.web { + br.Verify(t, tt.wantBrowserURL) + } reg.Verify(t) }) } From 97da7fc1d230f50e10f419df19faf6906be3670f Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Tue, 2 Sep 2025 12:50:15 -0600 Subject: [PATCH 2/9] Simplify BaseRepo assignment in list command Removes unnecessary nil check for Factory before assigning BaseRepo in the agent-task list command. This streamlines the code since Factory is always expected to be non-nil. --- pkg/cmd/agent-task/list/list.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/cmd/agent-task/list/list.go b/pkg/cmd/agent-task/list/list.go index 5b5ff0c38..6add74df8 100644 --- a/pkg/cmd/agent-task/list/list.go +++ b/pkg/cmd/agent-task/list/list.go @@ -46,9 +46,9 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, args []string) error { // Support -R/--repo override - if f != nil { - opts.BaseRepo = f.BaseRepo - } + + opts.BaseRepo = f.BaseRepo + if opts.Limit < 1 { return cmdutil.FlagErrorf("invalid limit: %v", opts.Limit) } From 4a07cdf94049791e670fac48f2cd71f0c7490a48 Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Tue, 2 Sep 2025 12:52:52 -0600 Subject: [PATCH 3/9] Simplify time calculation in listRun test Replaces explicit duration parsing with direct multiplication for calculating a timestamp 6 hours ago in Test_listRun, improving code clarity. --- pkg/cmd/agent-task/list/list_test.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/pkg/cmd/agent-task/list/list_test.go b/pkg/cmd/agent-task/list/list_test.go index a2c9d62ea..0df882b25 100644 --- a/pkg/cmd/agent-task/list/list_test.go +++ b/pkg/cmd/agent-task/list/list_test.go @@ -77,9 +77,7 @@ func TestNewCmdList(t *testing.T) { } func Test_listRun(t *testing.T) { - sixHours, _ := time.ParseDuration("6h") - sixHoursAgo := time.Now().Add(-sixHours) - createdAt := sixHoursAgo.Format(time.RFC3339) + createdAt := time.Now().Add(-6 * time.Hour).Format(time.RFC3339) // 6h ago tests := []struct { name string From 227f0bd01d7ca712f2bccb8644a6dd930d7376b3 Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Tue, 2 Sep 2025 12:55:03 -0600 Subject: [PATCH 4/9] Add comment explaining error handling in listRun Added a comment to clarify why the error from opts.BaseRepo() is ignored in listRun. This provides context for handling cases when the current working directory is not a repo and --repo is not set. --- pkg/cmd/agent-task/list/list.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/cmd/agent-task/list/list.go b/pkg/cmd/agent-task/list/list.go index 6add74df8..437767fe5 100644 --- a/pkg/cmd/agent-task/list/list.go +++ b/pkg/cmd/agent-task/list/list.go @@ -107,6 +107,8 @@ func listRun(opts *ListOptions) error { var repo ghrepo.Interface if opts.BaseRepo != nil { + // We swallow this error because when CWD is not a repo and + // the --repo flag is not set, we use the global/user session listing. repo, _ = opts.BaseRepo() } From 163eb7e7c88c0049d734fb28fe732137f6cf4060 Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Tue, 2 Sep 2025 13:04:27 -0600 Subject: [PATCH 5/9] Return NoResultsError when no agent tasks found Replaces plain output with a NoResultsError in listRun when no agent tasks are found. Updates related tests to expect the error instead of output, improving error handling consistency. --- pkg/cmd/agent-task/list/list.go | 3 +-- pkg/cmd/agent-task/list/list_test.go | 13 ++++--------- 2 files changed, 5 insertions(+), 11 deletions(-) diff --git a/pkg/cmd/agent-task/list/list.go b/pkg/cmd/agent-task/list/list.go index 437767fe5..896c0fadd 100644 --- a/pkg/cmd/agent-task/list/list.go +++ b/pkg/cmd/agent-task/list/list.go @@ -126,8 +126,7 @@ func listRun(opts *ListOptions) error { opts.IO.StopProgressIndicator() if len(sessions) == 0 { - fmt.Fprintln(opts.IO.Out, "no agent tasks found") - return nil + return cmdutil.NewNoResultsError("no agent tasks found") } cs := opts.IO.ColorScheme() diff --git a/pkg/cmd/agent-task/list/list_test.go b/pkg/cmd/agent-task/list/list_test.go index 0df882b25..a1545596a 100644 --- a/pkg/cmd/agent-task/list/list_test.go +++ b/pkg/cmd/agent-task/list/list_test.go @@ -96,7 +96,7 @@ func Test_listRun(t *testing.T) { name: "no sessions", tty: true, stubs: func(reg *httpmock.Registry) { registerEmptySessionsMock(reg) }, - wantOut: "no agent tasks found\n", + wantErr: cmdutil.NewNoResultsError("no agent tasks found"), }, { name: "limit truncates sessions", @@ -154,15 +154,14 @@ func Test_listRun(t *testing.T) { tty: true, stubs: func(reg *httpmock.Registry) { registerRepoEmptySessionsMock(reg, "OWNER", "REPO") }, baseRepo: ghrepo.New("OWNER", "REPO"), - wantOut: "no agent tasks found\n", + wantErr: cmdutil.NewNoResultsError("no agent tasks found"), }, { name: "repo resolution error does not surface", tty: true, baseRepoErr: errors.New("ambiguous repo"), - wantErr: nil, + wantErr: cmdutil.NewNoResultsError("no agent tasks found"), stubs: func(reg *httpmock.Registry) { registerEmptySessionsMock(reg) }, - wantOut: "no agent tasks found\n", }, { name: "repo scoped many sessions (tty)", @@ -238,11 +237,7 @@ func Test_listRun(t *testing.T) { } got := stdout.String() require.Equal(t, tt.wantOut, got) - if tt.wantStderr != "" { - require.Equal(t, tt.wantStderr, stderr.String()) - } else { - require.Equal(t, "", stderr.String()) - } + require.Equal(t, tt.wantStderr, stderr.String()) if tt.web { br.Verify(t, tt.wantBrowserURL) } From 51aa033b2e46cbe3999100d0c13f0fe3b061b622 Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Tue, 2 Sep 2025 13:09:12 -0600 Subject: [PATCH 6/9] Add test case for negative limit in list command Introduces a test to verify that passing a negative value to the --limit flag in the agent-task list command returns the expected error message. --- pkg/cmd/agent-task/list/list_test.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/pkg/cmd/agent-task/list/list_test.go b/pkg/cmd/agent-task/list/list_test.go index a1545596a..43884f553 100644 --- a/pkg/cmd/agent-task/list/list_test.go +++ b/pkg/cmd/agent-task/list/list_test.go @@ -45,6 +45,11 @@ func TestNewCmdList(t *testing.T) { args: "--limit 0", wantErr: "invalid limit: 0", }, + { + name: "negative limit", + args: "--limit -5", + wantErr: "invalid limit: -5", + }, { name: "web flag", args: "--web", From 93f7e9847f33d22451b4e3d3ba0b66b781309819 Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Tue, 2 Sep 2025 13:11:10 -0600 Subject: [PATCH 7/9] Add comment on web dashboard filtering limitation Added a comment explaining that the web GUI does not currently support filtering by repo, so the agents dashboard is opened without arguments. Notes future improvement if repo filtering becomes available. --- pkg/cmd/agent-task/list/list.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pkg/cmd/agent-task/list/list.go b/pkg/cmd/agent-task/list/list.go index 896c0fadd..0e3c43c08 100644 --- a/pkg/cmd/agent-task/list/list.go +++ b/pkg/cmd/agent-task/list/list.go @@ -84,6 +84,10 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman func listRun(opts *ListOptions) error { if opts.Web { + // Currently the web GUI does not have a page that supports filtering + // based on repo, so we just open the agents dashboard with no args. + // If that page is ever added in the future, we should route to that + // page instead of the global one when --repo is set. const webURL = "https://github.com/copilot/agents" if opts.IO.IsStdoutTTY() { fmt.Fprintf(opts.IO.ErrOut, "Opening %s in your browser.\n", text.DisplayURL(webURL)) From c4c0ddc8c265c82e077d21d05ad100cc2dbe7406 Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Tue, 2 Sep 2025 13:13:35 -0600 Subject: [PATCH 8/9] Add test for web mode with repo flag in listRun Added a test case to ensure that web mode in listRun uses the global URL even when the --repo flag is set. This improves coverage for scenarios where both web mode and repo are specified. --- pkg/cmd/agent-task/list/list_test.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/pkg/cmd/agent-task/list/list_test.go b/pkg/cmd/agent-task/list/list_test.go index 43884f553..a1c34ac32 100644 --- a/pkg/cmd/agent-task/list/list_test.go +++ b/pkg/cmd/agent-task/list/list_test.go @@ -191,6 +191,15 @@ func Test_listRun(t *testing.T) { wantStderr: "Opening https://github.com/copilot/agents in your browser.\n", wantBrowserURL: "https://github.com/copilot/agents", }, + { + name: "web mode with repo still uses global URL, even when --repo is set", + tty: true, + web: true, + baseRepo: ghrepo.New("OWNER", "REPO"), + wantOut: "", + wantStderr: "Opening https://github.com/copilot/agents in your browser.\n", + wantBrowserURL: "https://github.com/copilot/agents", + }, } for _, tt := range tests { From c5f7be9adbb77c75eae7f652a9f3cdbb9354f13c Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Tue, 2 Sep 2025 13:14:37 -0600 Subject: [PATCH 9/9] Replace panic with require.FailNow in test Updated the test to use require.FailNow instead of panic when CapiClient is called with --web, improving test failure reporting and consistency. --- pkg/cmd/agent-task/list/list_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/agent-task/list/list_test.go b/pkg/cmd/agent-task/list/list_test.go index a1c34ac32..ef3cf8a7b 100644 --- a/pkg/cmd/agent-task/list/list_test.go +++ b/pkg/cmd/agent-task/list/list_test.go @@ -231,7 +231,7 @@ func Test_listRun(t *testing.T) { Browser: br, CapiClient: func() (*capi.CAPIClient, error) { if tt.web { - panic("CapiClient should not be invoked when --web is set") + require.FailNow(t, "CapiClient was called with --web") } return capiClient, nil },