From 6b29c2905c7bcd589a1365d24be056daabc4b5c7 Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Wed, 10 Sep 2025 10:51:29 +0100 Subject: [PATCH 1/7] fix(agent-task/capi): also return PR URL from `GetPullRequestDatabaseID` Signed-off-by: Babak K. Shandiz --- pkg/cmd/agent-task/capi/client.go | 2 +- pkg/cmd/agent-task/capi/client_mock.go | 6 +++--- pkg/cmd/agent-task/capi/sessions.go | 11 ++++++----- pkg/cmd/agent-task/capi/sessions_test.go | 22 +++++++++++++--------- pkg/cmd/agent-task/view/view.go | 4 +++- 5 files changed, 26 insertions(+), 19 deletions(-) diff --git a/pkg/cmd/agent-task/capi/client.go b/pkg/cmd/agent-task/capi/client.go index 3e6d92736..45c214ea2 100644 --- a/pkg/cmd/agent-task/capi/client.go +++ b/pkg/cmd/agent-task/capi/client.go @@ -21,7 +21,7 @@ type CapiClient interface { GetJob(ctx context.Context, owner, repo, jobID string) (*Job, error) GetSession(ctx context.Context, id string) (*Session, error) ListSessionsByResourceID(ctx context.Context, resourceType string, resourceID int64, limit int) ([]*Session, error) - GetPullRequestDatabaseID(ctx context.Context, hostname string, owner string, repo string, number int) (int64, error) + GetPullRequestDatabaseID(ctx context.Context, hostname string, owner string, repo string, number int) (int64, string, error) } // CAPIClient is a client for interacting with the Copilot API diff --git a/pkg/cmd/agent-task/capi/client_mock.go b/pkg/cmd/agent-task/capi/client_mock.go index 7998f94d8..85b757943 100644 --- a/pkg/cmd/agent-task/capi/client_mock.go +++ b/pkg/cmd/agent-task/capi/client_mock.go @@ -24,7 +24,7 @@ var _ CapiClient = &CapiClientMock{} // GetJobFunc: func(ctx context.Context, owner string, repo string, jobID string) (*Job, error) { // panic("mock out the GetJob method") // }, -// GetPullRequestDatabaseIDFunc: func(ctx context.Context, hostname string, owner string, repo string, number int) (int64, error) { +// GetPullRequestDatabaseIDFunc: func(ctx context.Context, hostname string, owner string, repo string, number int) (int64, string, error) { // panic("mock out the GetPullRequestDatabaseID method") // }, // GetSessionFunc: func(ctx context.Context, id string) (*Session, error) { @@ -53,7 +53,7 @@ type CapiClientMock struct { GetJobFunc func(ctx context.Context, owner string, repo string, jobID string) (*Job, error) // GetPullRequestDatabaseIDFunc mocks the GetPullRequestDatabaseID method. - GetPullRequestDatabaseIDFunc func(ctx context.Context, hostname string, owner string, repo string, number int) (int64, error) + GetPullRequestDatabaseIDFunc func(ctx context.Context, hostname string, owner string, repo string, number int) (int64, string, error) // GetSessionFunc mocks the GetSession method. GetSessionFunc func(ctx context.Context, id string) (*Session, error) @@ -245,7 +245,7 @@ func (mock *CapiClientMock) GetJobCalls() []struct { } // GetPullRequestDatabaseID calls GetPullRequestDatabaseIDFunc. -func (mock *CapiClientMock) GetPullRequestDatabaseID(ctx context.Context, hostname string, owner string, repo string, number int) (int64, error) { +func (mock *CapiClientMock) GetPullRequestDatabaseID(ctx context.Context, hostname string, owner string, repo string, number int) (int64, string, error) { if mock.GetPullRequestDatabaseIDFunc == nil { panic("CapiClientMock.GetPullRequestDatabaseIDFunc: method is nil but CapiClient.GetPullRequestDatabaseID was just called") } diff --git a/pkg/cmd/agent-task/capi/sessions.go b/pkg/cmd/agent-task/capi/sessions.go index f6c5d7856..f28914f80 100644 --- a/pkg/cmd/agent-task/capi/sessions.go +++ b/pkg/cmd/agent-task/capi/sessions.go @@ -378,12 +378,13 @@ func (c *CAPIClient) hydrateSessionPullRequestsAndUsers(sessions []session) ([]* return newSessions, nil } -// GetPullRequestDatabaseID retrieves the database ID of a pull request given its number in a repository. -func (c *CAPIClient) GetPullRequestDatabaseID(ctx context.Context, hostname string, owner string, repo string, number int) (int64, error) { +// GetPullRequestDatabaseID retrieves the database ID and URL of a pull request given its number in a repository. +func (c *CAPIClient) GetPullRequestDatabaseID(ctx context.Context, hostname string, owner string, repo string, number int) (int64, string, error) { var resp struct { Repository struct { PullRequest struct { FullDatabaseID string `graphql:"fullDatabaseId"` + URL string `graphql:"url"` } `graphql:"pullRequest(number: $number)"` } `graphql:"repository(owner: $owner, name: $repo)"` } @@ -396,14 +397,14 @@ func (c *CAPIClient) GetPullRequestDatabaseID(ctx context.Context, hostname stri apiClient := api.NewClientFromHTTP(c.httpClient) if err := apiClient.Query(hostname, "GetPullRequestFullDatabaseID", &resp, variables); err != nil { - return 0, err + return 0, "", err } databaseID, err := strconv.ParseInt(resp.Repository.PullRequest.FullDatabaseID, 10, 64) if err != nil { - return 0, err + return 0, "", err } - return databaseID, nil + return databaseID, resp.Repository.PullRequest.URL, nil } // generatePullRequestNodeID converts an int64 databaseID and repoID to a GraphQL Node ID format diff --git a/pkg/cmd/agent-task/capi/sessions_test.go b/pkg/cmd/agent-task/capi/sessions_test.go index 1b750f56b..e67ed2ab1 100644 --- a/pkg/cmd/agent-task/capi/sessions_test.go +++ b/pkg/cmd/agent-task/capi/sessions_test.go @@ -1743,10 +1743,11 @@ func TestGetSession(t *testing.T) { } func TestGetPullRequestDatabaseID(t *testing.T) { tests := []struct { - name string - httpStubs func(*testing.T, *httpmock.Registry) - wantErr string - wantOut int64 + name string + httpStubs func(*testing.T, *httpmock.Registry) + wantErr string + wantDatabaseID int64 + wantURL string }{ { name: "graphql error", @@ -1764,24 +1765,26 @@ func TestGetPullRequestDatabaseID(t *testing.T) { httpStubs: func(t *testing.T, reg *httpmock.Registry) { reg.Register( httpmock.WithHost(httpmock.GraphQL(`query GetPullRequestFullDatabaseID\b`), "api.github.com"), - httpmock.StringResponse(`{"data": {"repository": {"pullRequest": {"fullDatabaseId": "non-int"}}}}`), + httpmock.StringResponse(`{"data": {"repository": {"pullRequest": {"fullDatabaseId": "non-int", "url": "some-url"}}}}`), ) }, wantErr: `strconv.ParseInt: parsing "non-int": invalid syntax`, + wantURL: "some-url", }, { name: "success", httpStubs: func(t *testing.T, reg *httpmock.Registry) { reg.Register( httpmock.WithHost(httpmock.GraphQL(`query GetPullRequestFullDatabaseID\b`), "api.github.com"), - httpmock.GraphQLQuery(`{"data": {"repository": {"pullRequest": {"fullDatabaseId": "999"}}}}`, func(s string, m map[string]interface{}) { + httpmock.GraphQLQuery(`{"data": {"repository": {"pullRequest": {"fullDatabaseId": "999", "url": "some-url"}}}}`, func(s string, m map[string]interface{}) { assert.Equal(t, "OWNER", m["owner"]) assert.Equal(t, "REPO", m["repo"]) assert.Equal(t, float64(42), m["number"]) }), ) }, - wantOut: 999, + wantDatabaseID: 999, + wantURL: "some-url", }, } @@ -1798,7 +1801,7 @@ func TestGetPullRequestDatabaseID(t *testing.T) { cfg := config.NewBlankConfig() capiClient := NewCAPIClient(httpClient, cfg.Authentication()) - databaseID, err := capiClient.GetPullRequestDatabaseID(context.Background(), "github.com", "OWNER", "REPO", 42) + databaseID, url, err := capiClient.GetPullRequestDatabaseID(context.Background(), "github.com", "OWNER", "REPO", 42) if tt.wantErr != "" { require.ErrorContains(t, err, tt.wantErr) @@ -1807,7 +1810,8 @@ func TestGetPullRequestDatabaseID(t *testing.T) { } require.NoError(t, err) - require.Equal(t, tt.wantOut, databaseID) + require.Equal(t, tt.wantDatabaseID, databaseID) + require.Equal(t, tt.wantURL, url) }) } } diff --git a/pkg/cmd/agent-task/view/view.go b/pkg/cmd/agent-task/view/view.go index c501f28b3..bac6ee936 100644 --- a/pkg/cmd/agent-task/view/view.go +++ b/pkg/cmd/agent-task/view/view.go @@ -109,6 +109,7 @@ func viewRun(opts *ViewOptions) error { } } else { var resourceID int64 + var prURL string if opts.SelectorArg != "" { // Finder does not support the PR/issue reference format (e.g. owner/repo#123) @@ -127,7 +128,7 @@ func viewRun(opts *ViewOptions) error { return fmt.Errorf("agent tasks are not supported on this host: %s", hostname) } - resourceID, err = capiClient.GetPullRequestDatabaseID(ctx, hostname, repo.RepoOwner(), repo.RepoName(), num) + resourceID, prURL, err = capiClient.GetPullRequestDatabaseID(ctx, hostname, repo.RepoOwner(), repo.RepoName(), num) if err != nil { return fmt.Errorf("failed to fetch pull request: %w", err) } @@ -155,6 +156,7 @@ func viewRun(opts *ViewOptions) error { } resourceID = databaseID + prURL = pr.URL } // TODO(babakks): currently we just fetch a pre-defined number of From ab7e4039daa1994b1dca60413636a92d5d72d448 Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Wed, 10 Sep 2025 10:54:40 +0100 Subject: [PATCH 2/7] refactor: move Copilot Agents home URL to capi Signed-off-by: Babak K. Shandiz --- pkg/cmd/agent-task/capi/sessions.go | 2 ++ pkg/cmd/agent-task/list/list.go | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/cmd/agent-task/capi/sessions.go b/pkg/cmd/agent-task/capi/sessions.go index f28914f80..aa7deabaf 100644 --- a/pkg/cmd/agent-task/capi/sessions.go +++ b/pkg/cmd/agent-task/capi/sessions.go @@ -18,6 +18,8 @@ import ( "github.com/vmihailenco/msgpack/v5" ) +const AgentsHomeURL = "https://github.com/copilot/agents" + var defaultSessionsPerPage = 50 var ErrSessionNotFound = errors.New("not found") diff --git a/pkg/cmd/agent-task/list/list.go b/pkg/cmd/agent-task/list/list.go index bff4ed835..5886955e6 100644 --- a/pkg/cmd/agent-task/list/list.go +++ b/pkg/cmd/agent-task/list/list.go @@ -70,7 +70,7 @@ func listRun(opts *ListOptions) error { // 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" + webURL := capi.AgentsHomeURL if opts.IO.IsStdoutTTY() { fmt.Fprintf(opts.IO.ErrOut, "Opening %s in your browser.\n", text.DisplayURL(webURL)) } From 57d9f1ff0e26f340377586ee40e421bceb53e583 Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Wed, 10 Sep 2025 10:55:05 +0100 Subject: [PATCH 3/7] feat(agent-task view): add `--web` mode Signed-off-by: Babak K. Shandiz --- pkg/cmd/agent-task/view/view.go | 44 ++++++++++++++++++++++++++++++--- 1 file changed, 41 insertions(+), 3 deletions(-) diff --git a/pkg/cmd/agent-task/view/view.go b/pkg/cmd/agent-task/view/view.go index bac6ee936..d02b8e710 100644 --- a/pkg/cmd/agent-task/view/view.go +++ b/pkg/cmd/agent-task/view/view.go @@ -10,6 +10,7 @@ import ( "time" "github.com/MakeNowJust/heredoc" + "github.com/cli/cli/v2/internal/browser" "github.com/cli/cli/v2/internal/ghinstance" "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/internal/prompter" @@ -31,10 +32,12 @@ type ViewOptions struct { HttpClient func() (*http.Client, error) Finder prShared.PRFinder Prompter prompter.Prompter + Browser browser.Browser SelectorArg string PRNumber int SessionID string + Web bool } func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Command { @@ -43,6 +46,7 @@ func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Comman HttpClient: f.HttpClient, CapiClient: shared.CapiClientFunc(f), Prompter: f.Prompter, + Browser: f.Browser, } cmd := &cobra.Command{ @@ -80,6 +84,8 @@ func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Comman cmdutil.EnableRepoOverride(cmd, f) + cmd.Flags().BoolVarP(&opts.Web, "web", "w", false, "Open agent task in the browser") + return cmd } @@ -98,15 +104,32 @@ func viewRun(opts *ViewOptions) error { var session *capi.Session if opts.SessionID != "" { - if sess, err := capiClient.GetSession(ctx, opts.SessionID); err != nil { + sess, err := capiClient.GetSession(ctx, opts.SessionID) + if err != nil { if errors.Is(err, capi.ErrSessionNotFound) { fmt.Fprintln(opts.IO.ErrOut, "session not found") return cmdutil.SilentError } return err - } else { - session = sess } + + if opts.Web { + var webURL string + if sess.PullRequest != nil { + webURL = fmt.Sprintf("%s/agent-sessions/%s", sess.PullRequest.URL, url.PathEscape(sess.ID)) + } else { + // Currently the web Copilot Agents home GUI does not support focusing + // on a given session, so we should just navigate to the home page. + webURL = capi.AgentsHomeURL + } + + if opts.IO.IsStdoutTTY() { + fmt.Fprintf(opts.IO.ErrOut, "Opening %s in your browser.\n", text.DisplayURL(webURL)) + } + return opts.Browser.Browse(webURL) + } + + session = sess } else { var resourceID int64 var prURL string @@ -173,6 +196,21 @@ func viewRun(opts *ViewOptions) error { return cmdutil.SilentError } + if opts.Web { + // Note that, we needed to make sure the PR exists and it has at least one session + // associated with it, other wise the `/agent-sessions` page would display the 404 + // error. + + // We don't need to navigate to a specific session; if there's only one session + // then the GUI will automatically show it, otherwise the user can select from the + // list. This is to avoid unnecessary prompting. + webURL := prURL + "/agent-sessions" + if opts.IO.IsStdoutTTY() { + fmt.Fprintf(opts.IO.ErrOut, "Opening %s in your browser.\n", text.DisplayURL(webURL)) + } + return opts.Browser.Browse(webURL) + } + session = sessions[0] if len(sessions) > 1 { now := time.Now() From ecbad7ecb9d537fe55640bc5d462ff747230f90b Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Wed, 10 Sep 2025 10:55:54 +0100 Subject: [PATCH 4/7] test(agent-task view): add `--web` mode tests Signed-off-by: Babak K. Shandiz --- pkg/cmd/agent-task/view/view_test.go | 330 ++++++++++++++++++++++++--- 1 file changed, 302 insertions(+), 28 deletions(-) diff --git a/pkg/cmd/agent-task/view/view_test.go b/pkg/cmd/agent-task/view/view_test.go index 64b63df5b..d825e996d 100644 --- a/pkg/cmd/agent-task/view/view_test.go +++ b/pkg/cmd/agent-task/view/view_test.go @@ -10,6 +10,7 @@ import ( "github.com/MakeNowJust/heredoc" "github.com/cli/cli/v2/api" + "github.com/cli/cli/v2/internal/browser" "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/internal/prompter" "github.com/cli/cli/v2/pkg/cmd/agent-task/capi" @@ -68,6 +69,15 @@ func TestNewCmdList(t *testing.T) { SelectorArg: "some-arg", }, }, + { + name: "web mode", + tty: true, + args: "some-arg -w", + wantOpts: ViewOptions{ + SelectorArg: "some-arg", + Web: true, + }, + }, } for _, tt := range tests { @@ -116,14 +126,15 @@ func Test_viewRun(t *testing.T) { sampleDate := time.Now().Add(-6 * time.Hour) // 6h ago tests := []struct { - name string - tty bool - opts ViewOptions - promptStubs func(*testing.T, *prompter.MockPrompter) - capiStubs func(*testing.T, *capi.CapiClientMock) - wantOut string - wantErr error - wantStderr string + name string + tty bool + opts ViewOptions + promptStubs func(*testing.T, *prompter.MockPrompter) + capiStubs func(*testing.T, *capi.CapiClientMock) + wantOut string + wantErr error + wantStderr string + wantBrowserURL string }{ { name: "with session id, not found (tty)", @@ -273,6 +284,74 @@ func Test_viewRun(t *testing.T) { Started about 6 hours ago `), }, + { + name: "with session id, not found, web mode (tty)", + tty: true, + opts: ViewOptions{ + SelectorArg: "some-session-id", + SessionID: "some-session-id", + Web: true, + }, + capiStubs: func(t *testing.T, m *capi.CapiClientMock) { + m.GetSessionFunc = func(_ context.Context, _ string) (*capi.Session, error) { + return nil, capi.ErrSessionNotFound + } + }, + wantStderr: "session not found\n", + wantErr: cmdutil.SilentError, + }, + { + name: "with session id, without pr data, web mode (tty)", + tty: true, + opts: ViewOptions{ + SelectorArg: "some-session-id", + SessionID: "some-session-id", + Web: true, + }, + capiStubs: func(t *testing.T, m *capi.CapiClientMock) { + m.GetSessionFunc = func(_ context.Context, id string) (*capi.Session, error) { + assert.Equal(t, "some-session-id", id) + return &capi.Session{ + ID: "some-session-id", + State: "completed", + CreatedAt: sampleDate, + // User data is irrelevant in this case + }, nil + } + }, + wantBrowserURL: "https://github.com/copilot/agents", + wantStderr: "Opening https://github.com/copilot/agents in your browser.\n", + }, + { + name: "with session id, with pr data, web mode (tty)", + tty: true, + opts: ViewOptions{ + SelectorArg: "some-session-id", + SessionID: "some-session-id", + Web: true, + }, + capiStubs: func(t *testing.T, m *capi.CapiClientMock) { + m.GetSessionFunc = func(_ context.Context, id string) (*capi.Session, error) { + assert.Equal(t, "some-session-id", id) + return &capi.Session{ + ID: "some-session-id", + State: "completed", + CreatedAt: sampleDate, + PullRequest: &api.PullRequest{ + Title: "fix something", + Number: 101, + URL: "https://github.com/OWNER/REPO/pull/101", + Repository: &api.PRRepository{ + NameWithOwner: "OWNER/REPO", + }, + }, + // User data is irrelevant in this case + }, nil + } + }, + wantBrowserURL: "https://github.com/OWNER/REPO/pull/101/agent-sessions/some-session-id", + wantStderr: "Opening https://github.com/OWNER/REPO/pull/101/agent-sessions/some-session-id in your browser.\n", + }, { name: "with pr number, api error (tty)", tty: true, @@ -280,7 +359,10 @@ func Test_viewRun(t *testing.T) { SelectorArg: "pr-number", Finder: prShared.NewMockFinder( "pr-number", - &api.PullRequest{FullDatabaseID: "999999"}, + &api.PullRequest{ + FullDatabaseID: "999999", + URL: "https://github.com/OWNER/REPO/pull/101", + }, ghrepo.New("OWNER", "REPO"), ), }, @@ -312,8 +394,8 @@ func Test_viewRun(t *testing.T) { }, }, capiStubs: func(t *testing.T, m *capi.CapiClientMock) { - m.GetPullRequestDatabaseIDFunc = func(_ context.Context, _ string, _ string, _ string, _ int) (int64, error) { - return 0, errors.New("some error") + m.GetPullRequestDatabaseIDFunc = func(_ context.Context, _ string, _ string, _ string, _ int) (int64, string, error) { + return 0, "", errors.New("some error") } }, wantErr: errors.New("failed to fetch pull request: some error"), @@ -328,8 +410,8 @@ func Test_viewRun(t *testing.T) { }, }, capiStubs: func(t *testing.T, m *capi.CapiClientMock) { - m.GetPullRequestDatabaseIDFunc = func(_ context.Context, _ string, _ string, _ string, _ int) (int64, error) { - return 999999, nil + m.GetPullRequestDatabaseIDFunc = func(_ context.Context, _ string, _ string, _ string, _ int) (int64, string, error) { + return 999999, "some-url", nil } m.ListSessionsByResourceIDFunc = func(_ context.Context, _ string, _ int64, _ int) ([]*capi.Session, error) { return nil, errors.New("some error") @@ -344,7 +426,10 @@ func Test_viewRun(t *testing.T) { SelectorArg: "pr-number", Finder: prShared.NewMockFinder( "pr-number", - &api.PullRequest{FullDatabaseID: "999999"}, + &api.PullRequest{ + FullDatabaseID: "999999", + URL: "https://github.com/OWNER/REPO/pull/101", + }, ghrepo.New("OWNER", "REPO"), ), }, @@ -388,7 +473,10 @@ func Test_viewRun(t *testing.T) { SelectorArg: "pr-number", Finder: prShared.NewMockFinder( "pr-number", - &api.PullRequest{FullDatabaseID: "999999"}, + &api.PullRequest{ + FullDatabaseID: "999999", + URL: "https://github.com/OWNER/REPO/pull/101", + }, ghrepo.New("OWNER", "REPO"), ), }, @@ -421,9 +509,9 @@ func Test_viewRun(t *testing.T) { State: "completed", CreatedAt: sampleDate, PullRequest: &api.PullRequest{ - Title: "fix something else", - Number: 102, - URL: "https://github.com/OWNER/REPO/pull/102", + Title: "fix something", + Number: 101, + URL: "https://github.com/OWNER/REPO/pull/101", Repository: &api.PRRepository{ NameWithOwner: "OWNER/REPO", }, @@ -459,18 +547,18 @@ func Test_viewRun(t *testing.T) { name: "with pr reference, success, multiple sessions with pr and user data (tty)", tty: true, opts: ViewOptions{ - SelectorArg: "OWNER/REPO#999", + SelectorArg: "OWNER/REPO#101", BaseRepo: func() (ghrepo.Interface, error) { return ghrepo.New("OWNER", "REPO"), nil }, }, capiStubs: func(t *testing.T, m *capi.CapiClientMock) { - m.GetPullRequestDatabaseIDFunc = func(_ context.Context, hostname string, owner string, repo string, number int) (int64, error) { + m.GetPullRequestDatabaseIDFunc = func(_ context.Context, hostname string, owner string, repo string, number int) (int64, string, error) { assert.Equal(t, "github.com", hostname) assert.Equal(t, "OWNER", owner) assert.Equal(t, "REPO", repo) - assert.Equal(t, 999, number) - return 999999, nil + assert.Equal(t, 101, number) + return 999999, "https://github.com/OWNER/REPO/pull/101", nil } m.ListSessionsByResourceIDFunc = func(_ context.Context, resourceType string, resourceID int64, limit int) ([]*capi.Session, error) { assert.Equal(t, "pull", resourceType) @@ -500,9 +588,9 @@ func Test_viewRun(t *testing.T) { State: "completed", CreatedAt: sampleDate, PullRequest: &api.PullRequest{ - Title: "fix something else", - Number: 102, - URL: "https://github.com/OWNER/REPO/pull/102", + Title: "fix something", + Number: 101, + URL: "https://github.com/OWNER/REPO/pull/101", Repository: &api.PRRepository{ NameWithOwner: "OWNER/REPO", }, @@ -534,6 +622,189 @@ func Test_viewRun(t *testing.T) { https://github.com/OWNER/REPO/pull/101/agent-sessions/some-session-id `), }, + { + name: "with pr number, api error, web mode (tty)", + tty: true, + opts: ViewOptions{ + SelectorArg: "pr-number", + Finder: prShared.NewMockFinder( + "pr-number", + &api.PullRequest{ + FullDatabaseID: "999999", + URL: "https://github.com/OWNER/REPO/pull/101", + }, + ghrepo.New("OWNER", "REPO"), + ), + Web: true, + }, + capiStubs: func(t *testing.T, m *capi.CapiClientMock) { + m.ListSessionsByResourceIDFunc = func(_ context.Context, _ string, _ int64, _ int) ([]*capi.Session, error) { + return nil, errors.New("some error") + } + }, + wantErr: errors.New("failed to list sessions for pull request: some error"), + }, + { + name: "with pr number, single session with pr data, web mode (tty)", + tty: true, + opts: ViewOptions{ + SelectorArg: "pr-number", + Finder: prShared.NewMockFinder( + "pr-number", + &api.PullRequest{ + FullDatabaseID: "999999", + URL: "https://github.com/OWNER/REPO/pull/101", + }, + ghrepo.New("OWNER", "REPO"), + ), + Web: true, + }, + capiStubs: func(t *testing.T, m *capi.CapiClientMock) { + m.ListSessionsByResourceIDFunc = func(_ context.Context, resourceType string, resourceID int64, limit int) ([]*capi.Session, error) { + assert.Equal(t, "pull", resourceType) + assert.Equal(t, int64(999999), resourceID) + assert.Equal(t, defaultLimit, limit) + return []*capi.Session{ + { + ID: "some-session-id", + State: "completed", + CreatedAt: sampleDate, + PullRequest: &api.PullRequest{ + Title: "fix something", + Number: 101, + URL: "https://github.com/OWNER/REPO/pull/101", + Repository: &api.PRRepository{ + NameWithOwner: "OWNER/REPO", + }, + }, + // User data is irrelevant in this case + }, + }, nil + } + }, + wantBrowserURL: "https://github.com/OWNER/REPO/pull/101/agent-sessions", + wantStderr: "Opening https://github.com/OWNER/REPO/pull/101/agent-sessions in your browser.\n", + }, + { + name: "with pr number, multiple session with pr data, web mode (tty)", + tty: true, + opts: ViewOptions{ + SelectorArg: "pr-number", + Finder: prShared.NewMockFinder( + "pr-number", + &api.PullRequest{ + FullDatabaseID: "999999", + URL: "https://github.com/OWNER/REPO/pull/101", + }, + ghrepo.New("OWNER", "REPO"), + ), + Web: true, + }, + capiStubs: func(t *testing.T, m *capi.CapiClientMock) { + m.ListSessionsByResourceIDFunc = func(_ context.Context, resourceType string, resourceID int64, limit int) ([]*capi.Session, error) { + assert.Equal(t, "pull", resourceType) + assert.Equal(t, int64(999999), resourceID) + assert.Equal(t, defaultLimit, limit) + return []*capi.Session{ + { + ID: "some-session-id", + Name: "session one", + State: "completed", + CreatedAt: sampleDate, + PullRequest: &api.PullRequest{ + Title: "fix something", + Number: 101, + URL: "https://github.com/OWNER/REPO/pull/101", + Repository: &api.PRRepository{ + NameWithOwner: "OWNER/REPO", + }, + }, + // User data is irrelevant in this case + }, + { + ID: "some-other-session-id", + Name: "session two", + State: "completed", + CreatedAt: sampleDate, + PullRequest: &api.PullRequest{ + Title: "fix something", + Number: 101, + URL: "https://github.com/OWNER/REPO/pull/101", + Repository: &api.PRRepository{ + NameWithOwner: "OWNER/REPO", + }, + }, + // User data is irrelevant in this case + }, + }, nil + } + }, + wantBrowserURL: "https://github.com/OWNER/REPO/pull/101/agent-sessions", + wantStderr: "Opening https://github.com/OWNER/REPO/pull/101/agent-sessions in your browser.\n", + }, + { + name: "with pr reference, multiple sessions with pr and user data, web mode (tty)", + tty: true, + opts: ViewOptions{ + SelectorArg: "OWNER/REPO#101", + BaseRepo: func() (ghrepo.Interface, error) { + return ghrepo.New("OWNER", "REPO"), nil + }, + Web: true, + }, + capiStubs: func(t *testing.T, m *capi.CapiClientMock) { + m.GetPullRequestDatabaseIDFunc = func(_ context.Context, hostname string, owner string, repo string, number int) (int64, string, error) { + assert.Equal(t, "github.com", hostname) + assert.Equal(t, "OWNER", owner) + assert.Equal(t, "REPO", repo) + assert.Equal(t, 101, number) + return 999999, "https://github.com/OWNER/REPO/pull/101", nil + } + m.ListSessionsByResourceIDFunc = func(_ context.Context, resourceType string, resourceID int64, limit int) ([]*capi.Session, error) { + assert.Equal(t, "pull", resourceType) + assert.Equal(t, int64(999999), resourceID) + assert.Equal(t, defaultLimit, limit) + return []*capi.Session{ + { + ID: "some-session-id", + Name: "session one", + State: "completed", + CreatedAt: sampleDate, + PullRequest: &api.PullRequest{ + Title: "fix something", + Number: 101, + URL: "https://github.com/OWNER/REPO/pull/101", + Repository: &api.PRRepository{ + NameWithOwner: "OWNER/REPO", + }, + }, + User: &api.GitHubUser{ + Login: "octocat", + }, + }, + { + ID: "some-other-session-id", + Name: "session two", + State: "completed", + CreatedAt: sampleDate, + PullRequest: &api.PullRequest{ + Title: "fix something", + Number: 101, + URL: "https://github.com/OWNER/REPO/pull/101", + Repository: &api.PRRepository{ + NameWithOwner: "OWNER/REPO", + }, + }, + User: &api.GitHubUser{ + Login: "octocat", + }, + }, + }, nil + } + }, + wantBrowserURL: "https://github.com/OWNER/REPO/pull/101/agent-sessions", + wantStderr: "Opening https://github.com/OWNER/REPO/pull/101/agent-sessions in your browser.\n", + }, } for _, tt := range tests { @@ -551,9 +822,12 @@ func Test_viewRun(t *testing.T) { ios, _, stdout, stderr := iostreams.Test() ios.SetStdoutTTY(tt.tty) + browser := &browser.Stub{} + opts := tt.opts opts.IO = ios opts.Prompter = prompter + opts.Browser = browser opts.CapiClient = func() (capi.CapiClient, error) { return capiClientMock, nil } @@ -566,9 +840,9 @@ func Test_viewRun(t *testing.T) { require.NoError(t, err) } - got := stdout.String() - require.Equal(t, tt.wantOut, got) - require.Equal(t, tt.wantStderr, stderr.String()) + assert.Equal(t, tt.wantOut, stdout.String()) + assert.Equal(t, tt.wantStderr, stderr.String()) + assert.Equal(t, tt.wantBrowserURL, browser.BrowsedURL()) }) } } From b3b8697cf34cad96617bd97adbcc59d762e4e580 Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Wed, 10 Sep 2025 11:04:44 +0100 Subject: [PATCH 5/7] test(agent-task view): enhance naming and stubs Signed-off-by: Babak K. Shandiz --- pkg/cmd/agent-task/view/view_test.go | 50 +++++++++++++--------------- 1 file changed, 23 insertions(+), 27 deletions(-) diff --git a/pkg/cmd/agent-task/view/view_test.go b/pkg/cmd/agent-task/view/view_test.go index d825e996d..a4636c369 100644 --- a/pkg/cmd/agent-task/view/view_test.go +++ b/pkg/cmd/agent-task/view/view_test.go @@ -356,9 +356,9 @@ func Test_viewRun(t *testing.T) { name: "with pr number, api error (tty)", tty: true, opts: ViewOptions{ - SelectorArg: "pr-number", + SelectorArg: "101", Finder: prShared.NewMockFinder( - "pr-number", + "101", &api.PullRequest{ FullDatabaseID: "999999", URL: "https://github.com/OWNER/REPO/pull/101", @@ -377,7 +377,7 @@ func Test_viewRun(t *testing.T) { name: "with pr reference, unsupported hostname (tty)", tty: true, opts: ViewOptions{ - SelectorArg: "OWNER/REPO#999", + SelectorArg: "OWNER/REPO#101", BaseRepo: func() (ghrepo.Interface, error) { return ghrepo.NewWithHost("OWNER", "REPO", "foo.com"), nil }, @@ -388,7 +388,7 @@ func Test_viewRun(t *testing.T) { name: "with pr reference, api error when fetching pr database ID (tty)", tty: true, opts: ViewOptions{ - SelectorArg: "OWNER/REPO#999", + SelectorArg: "OWNER/REPO#101", BaseRepo: func() (ghrepo.Interface, error) { return ghrepo.New("OWNER", "REPO"), nil }, @@ -404,7 +404,7 @@ func Test_viewRun(t *testing.T) { name: "with pr reference, api error when fetching session (tty)", tty: true, opts: ViewOptions{ - SelectorArg: "OWNER/REPO#999", + SelectorArg: "OWNER/REPO#101", BaseRepo: func() (ghrepo.Interface, error) { return ghrepo.New("OWNER", "REPO"), nil }, @@ -420,12 +420,12 @@ func Test_viewRun(t *testing.T) { wantErr: errors.New("failed to list sessions for pull request: some error"), }, { - name: "with pr number, success, single session with pr and user data (tty)", + name: "with pr number, success, single session (tty)", tty: true, opts: ViewOptions{ - SelectorArg: "pr-number", + SelectorArg: "101", Finder: prShared.NewMockFinder( - "pr-number", + "101", &api.PullRequest{ FullDatabaseID: "999999", URL: "https://github.com/OWNER/REPO/pull/101", @@ -467,12 +467,12 @@ func Test_viewRun(t *testing.T) { `), }, { - name: "with pr number, success, multiple sessions with pr and user data (tty)", + name: "with pr number, success, multiple sessions (tty)", tty: true, opts: ViewOptions{ - SelectorArg: "pr-number", + SelectorArg: "101", Finder: prShared.NewMockFinder( - "pr-number", + "101", &api.PullRequest{ FullDatabaseID: "999999", URL: "https://github.com/OWNER/REPO/pull/101", @@ -544,7 +544,7 @@ func Test_viewRun(t *testing.T) { `), }, { - name: "with pr reference, success, multiple sessions with pr and user data (tty)", + name: "with pr reference, success, multiple sessions (tty)", tty: true, opts: ViewOptions{ SelectorArg: "OWNER/REPO#101", @@ -626,9 +626,9 @@ func Test_viewRun(t *testing.T) { name: "with pr number, api error, web mode (tty)", tty: true, opts: ViewOptions{ - SelectorArg: "pr-number", + SelectorArg: "101", Finder: prShared.NewMockFinder( - "pr-number", + "101", &api.PullRequest{ FullDatabaseID: "999999", URL: "https://github.com/OWNER/REPO/pull/101", @@ -645,12 +645,12 @@ func Test_viewRun(t *testing.T) { wantErr: errors.New("failed to list sessions for pull request: some error"), }, { - name: "with pr number, single session with pr data, web mode (tty)", + name: "with pr number, single session, web mode (tty)", tty: true, opts: ViewOptions{ - SelectorArg: "pr-number", + SelectorArg: "101", Finder: prShared.NewMockFinder( - "pr-number", + "101", &api.PullRequest{ FullDatabaseID: "999999", URL: "https://github.com/OWNER/REPO/pull/101", @@ -686,12 +686,12 @@ func Test_viewRun(t *testing.T) { wantStderr: "Opening https://github.com/OWNER/REPO/pull/101/agent-sessions in your browser.\n", }, { - name: "with pr number, multiple session with pr data, web mode (tty)", + name: "with pr number, multiple sessions, web mode (tty)", tty: true, opts: ViewOptions{ - SelectorArg: "pr-number", + SelectorArg: "101", Finder: prShared.NewMockFinder( - "pr-number", + "101", &api.PullRequest{ FullDatabaseID: "999999", URL: "https://github.com/OWNER/REPO/pull/101", @@ -743,7 +743,7 @@ func Test_viewRun(t *testing.T) { wantStderr: "Opening https://github.com/OWNER/REPO/pull/101/agent-sessions in your browser.\n", }, { - name: "with pr reference, multiple sessions with pr and user data, web mode (tty)", + name: "with pr reference, multiple sessions, web mode (tty)", tty: true, opts: ViewOptions{ SelectorArg: "OWNER/REPO#101", @@ -778,9 +778,7 @@ func Test_viewRun(t *testing.T) { NameWithOwner: "OWNER/REPO", }, }, - User: &api.GitHubUser{ - Login: "octocat", - }, + // User data is irrelevant in this case }, { ID: "some-other-session-id", @@ -795,9 +793,7 @@ func Test_viewRun(t *testing.T) { NameWithOwner: "OWNER/REPO", }, }, - User: &api.GitHubUser{ - Login: "octocat", - }, + // User data is irrelevant in this case }, }, nil } From 88e2d0d7d9343ef4491a84c5eded2827cd19b727 Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Wed, 10 Sep 2025 11:41:49 +0100 Subject: [PATCH 6/7] fix(agent-task view): stop progress indicator before opening browser Signed-off-by: Babak K. Shandiz --- pkg/cmd/agent-task/view/view.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/pkg/cmd/agent-task/view/view.go b/pkg/cmd/agent-task/view/view.go index d02b8e710..b592af7cf 100644 --- a/pkg/cmd/agent-task/view/view.go +++ b/pkg/cmd/agent-task/view/view.go @@ -113,6 +113,8 @@ func viewRun(opts *ViewOptions) error { return err } + opts.IO.StopProgressIndicator() + if opts.Web { var webURL string if sess.PullRequest != nil { @@ -196,6 +198,8 @@ func viewRun(opts *ViewOptions) error { return cmdutil.SilentError } + opts.IO.StopProgressIndicator() + if opts.Web { // Note that, we needed to make sure the PR exists and it has at least one session // associated with it, other wise the `/agent-sessions` page would display the 404 @@ -224,7 +228,6 @@ func viewRun(opts *ViewOptions) error { )) } - opts.IO.StopProgressIndicator() selected, err := opts.Prompter.Select("Select a session", "", options) if err != nil { return err @@ -234,8 +237,6 @@ func viewRun(opts *ViewOptions) error { } } - opts.IO.StopProgressIndicator() - out := opts.IO.Out if session.PullRequest != nil { From 6a5481368f1db90897adfc03a3b9e77cc33ba3bf Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Wed, 10 Sep 2025 13:53:45 +0100 Subject: [PATCH 7/7] docs(agent-task view): improve `--help` docs Signed-off-by: Babak K. Shandiz --- pkg/cmd/agent-task/view/view.go | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/pkg/cmd/agent-task/view/view.go b/pkg/cmd/agent-task/view/view.go index b592af7cf..787157ffe 100644 --- a/pkg/cmd/agent-task/view/view.go +++ b/pkg/cmd/agent-task/view/view.go @@ -51,10 +51,26 @@ func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Comman cmd := &cobra.Command{ Use: "view [ | | | ]", - Short: "View an agent task session", + Short: "View an agent task session (preview)", Long: heredoc.Doc(` View an agent task session. `), + Example: heredoc.Doc(` + # View an agent task by session ID + $ gh agent-task view e2fa49d2-f164-4a56-ab99-498090b8fcdf + + # View an agent task by pull request number in current repo + $ gh agent-task view 12345 + + # View an agent task by pull request number + $ gh agent-task view --repo OWNER/REPO 12345 + + # View an agent task by pull request reference + $ gh agent-task view OWNER/REPO#12345 + + # View a pull request agents tasks in the browser + $ gh agent-task view 12345 --web + `), Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { // Support -R/--repo override