From 1ccbb0af83dc3d0316a6a69b10e60b6e2ae4bb7e Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Fri, 12 Sep 2025 13:34:18 +0100 Subject: [PATCH] feat(agent-task view): add `--log` and `--follow` flags Signed-off-by: Babak K. Shandiz --- pkg/cmd/agent-task/view/view.go | 101 +++++++++++++--- pkg/cmd/agent-task/view/view_test.go | 170 +++++++++++++++++++++++++-- 2 files changed, 244 insertions(+), 27 deletions(-) diff --git a/pkg/cmd/agent-task/view/view.go b/pkg/cmd/agent-task/view/view.go index 024101b0e..9058ec83e 100644 --- a/pkg/cmd/agent-task/view/view.go +++ b/pkg/cmd/agent-task/view/view.go @@ -23,7 +23,10 @@ import ( "github.com/spf13/cobra" ) -const defaultLimit = 40 +const ( + defaultLimit = 40 + defaultLogPollInterval = 5 * time.Second +) type ViewOptions struct { IO *iostreams.IOStreams @@ -34,19 +37,30 @@ type ViewOptions struct { Prompter prompter.Prompter Browser browser.Browser + LogRenderer func() shared.LogRenderer + Sleep func(d time.Duration) + SelectorArg string PRNumber int SessionID string Web bool + Log bool + Follow bool +} + +func defaultLogRenderer() shared.LogRenderer { + return shared.NewLogRenderer() } func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Command { opts := &ViewOptions{ - IO: f.IOStreams, - HttpClient: f.HttpClient, - CapiClient: shared.CapiClientFunc(f), - Prompter: f.Prompter, - Browser: f.Browser, + IO: f.IOStreams, + HttpClient: f.HttpClient, + CapiClient: shared.CapiClientFunc(f), + Prompter: f.Prompter, + Browser: f.Browser, + LogRenderer: defaultLogRenderer, + Sleep: time.Sleep, } cmd := &cobra.Command{ @@ -89,6 +103,10 @@ func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Comman return fmt.Errorf("session ID is required when not running interactively") } + if opts.Follow && !opts.Log { + return cmdutil.FlagErrorf("--log is required when providing --follow") + } + if opts.Finder == nil { opts.Finder = prShared.NewFinder(f) } @@ -103,6 +121,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") + cmd.Flags().BoolVar(&opts.Log, "log", false, "Show agent session logs") + cmd.Flags().BoolVar(&opts.Follow, "follow", false, "Follow agent session logs") return cmd } @@ -255,10 +275,19 @@ func viewRun(opts *ViewOptions) error { } } - out := opts.IO.Out + printSession(opts, session) + + if opts.Log { + return printLogs(opts, capiClient, session.ID) + } + return nil +} + +func printSession(opts *ViewOptions, session *capi.Session) { + cs := opts.IO.ColorScheme() if session.PullRequest != nil { - fmt.Fprintf(out, "%s • %s • %s%s\n", + fmt.Fprintf(opts.IO.Out, "%s • %s • %s%s\n", shared.ColorFuncForSessionState(*session, cs)(shared.SessionStateString(session.State)), cs.Bold(session.PullRequest.Title), session.PullRequest.Repository.NameWithOwner, @@ -266,25 +295,61 @@ func viewRun(opts *ViewOptions) error { ) } else { // This can happen when the session is just created and a PR is not yet available for it - fmt.Fprintf(out, "%s\n", shared.ColorFuncForSessionState(*session, cs)(shared.SessionStateString(session.State))) + fmt.Fprintf(opts.IO.Out, "%s\n", shared.ColorFuncForSessionState(*session, cs)(shared.SessionStateString(session.State))) } if session.User != nil { - fmt.Fprintf(out, "Started on behalf of %s %s\n", session.User.Login, text.FuzzyAgo(time.Now(), session.CreatedAt)) + fmt.Fprintf(opts.IO.Out, "Started on behalf of %s %s\n", session.User.Login, text.FuzzyAgo(time.Now(), session.CreatedAt)) } else { // Should never happen, but we need to cover the path - fmt.Fprintf(out, "Started %s\n", text.FuzzyAgo(time.Now(), session.CreatedAt)) + fmt.Fprintf(opts.IO.Out, "Started %s\n", text.FuzzyAgo(time.Now(), session.CreatedAt)) } - // TODO(babakks): uncomment when we have the --logs option ready - // fmt.Fprintln(out, "") - // fmt.Fprintf(out, "For the detailed session logs, try: gh agent-task view '%s' --logs\n", opts.SelectorArg) + if !opts.Log { + fmt.Fprintln(opts.IO.Out, "") + fmt.Fprintf(opts.IO.Out, "For detailed session logs, try:\ngh agent-task view '%s' --log\n", session.ID) + } else if !opts.Follow { + fmt.Fprintln(opts.IO.Out, "") + fmt.Fprintf(opts.IO.Out, "To follow session logs, try:\ngh agent-task view '%s' --log --follow\n", session.ID) + } if session.PullRequest != nil { - fmt.Fprintln(out, "") - fmt.Fprintln(out, cs.Muted("View this session on GitHub:")) - fmt.Fprintln(out, cs.Muted(fmt.Sprintf("%s/agent-sessions/%s", session.PullRequest.URL, url.PathEscape(session.ID)))) + fmt.Fprintln(opts.IO.Out, "") + fmt.Fprintln(opts.IO.Out, cs.Muted("View this session on GitHub:")) + fmt.Fprintln(opts.IO.Out, cs.Muted(fmt.Sprintf("%s/agent-sessions/%s", session.PullRequest.URL, url.PathEscape(session.ID)))) + } +} + +func printLogs(opts *ViewOptions, capiClient capi.CapiClient, sessionID string) error { + ctx := context.Background() + + cs := opts.IO.ColorScheme() + renderer := opts.LogRenderer() + + if opts.Follow { + var called bool + fetcher := func() ([]byte, error) { + if called { + opts.Sleep(defaultLogPollInterval) + } + called = true + raw, err := capiClient.GetSessionLogs(ctx, sessionID) + if err != nil { + return nil, err + } + return raw, nil + } + + fmt.Fprintln(opts.IO.Out, "") + return renderer.Follow(fetcher, opts.IO.Out, cs) } - return nil + raw, err := capiClient.GetSessionLogs(ctx, sessionID) + if err != nil { + return fmt.Errorf("failed to fetch session logs: %w", err) + } + + fmt.Fprintln(opts.IO.Out, "") + _, err = renderer.Render(raw, opts.IO.Out, cs) + return err } diff --git a/pkg/cmd/agent-task/view/view_test.go b/pkg/cmd/agent-task/view/view_test.go index bdff45793..dd361f919 100644 --- a/pkg/cmd/agent-task/view/view_test.go +++ b/pkg/cmd/agent-task/view/view_test.go @@ -14,6 +14,7 @@ import ( "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/internal/prompter" "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" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/iostreams" @@ -78,6 +79,31 @@ func TestNewCmdList(t *testing.T) { SelectorArg: "some-arg", }, }, + { + name: "with --log", + tty: true, + args: "some-arg --log", + wantOpts: ViewOptions{ + SelectorArg: "some-arg", + Log: true, + }, + }, + { + name: "with --log and --follow", + tty: true, + args: "some-arg --log --follow", + wantOpts: ViewOptions{ + SelectorArg: "some-arg", + Log: true, + Follow: true, + }, + }, + { + name: "--follow requires --log", + tty: true, + args: "some-arg --follow", + wantErr: "--log is required when providing --follow", + }, { name: "web mode", tty: true, @@ -135,15 +161,16 @@ 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 - wantBrowserURL string + name string + tty bool + opts ViewOptions + promptStubs func(*testing.T, *prompter.MockPrompter) + capiStubs func(*testing.T, *capi.CapiClientMock) + logRendererStubs func(*testing.T, *shared.LogRendererMock) + wantOut string + wantErr error + wantStderr string + wantBrowserURL string }{ { name: "with session id, not found (tty)", @@ -206,6 +233,9 @@ func Test_viewRun(t *testing.T) { Completed • fix something • OWNER/REPO#101 Started on behalf of octocat about 6 hours ago + For detailed session logs, try: + gh agent-task view 'some-session-id' --log + View this session on GitHub: https://github.com/OWNER/REPO/pull/101/agent-sessions/some-session-id `), @@ -240,6 +270,9 @@ func Test_viewRun(t *testing.T) { Completed • fix something • OWNER/REPO#101 Started about 6 hours ago + For detailed session logs, try: + gh agent-task view 'some-session-id' --log + View this session on GitHub: https://github.com/OWNER/REPO/pull/101/agent-sessions/some-session-id `), @@ -268,6 +301,9 @@ func Test_viewRun(t *testing.T) { wantOut: heredoc.Doc(` Completed Started on behalf of octocat about 6 hours ago + + For detailed session logs, try: + gh agent-task view 'some-session-id' --log `), }, { @@ -291,6 +327,9 @@ func Test_viewRun(t *testing.T) { wantOut: heredoc.Doc(` Completed Started about 6 hours ago + + For detailed session logs, try: + gh agent-task view 'some-session-id' --log `), }, { @@ -471,6 +510,9 @@ func Test_viewRun(t *testing.T) { Completed • fix something • OWNER/REPO#101 Started on behalf of octocat about 6 hours ago + For detailed session logs, try: + gh agent-task view 'some-session-id' --log + View this session on GitHub: https://github.com/OWNER/REPO/pull/101/agent-sessions/some-session-id `), @@ -548,6 +590,9 @@ func Test_viewRun(t *testing.T) { Completed • fix something • OWNER/REPO#101 Started on behalf of octocat about 6 hours ago + For detailed session logs, try: + gh agent-task view 'some-session-id' --log + View this session on GitHub: https://github.com/OWNER/REPO/pull/101/agent-sessions/some-session-id `), @@ -627,6 +672,9 @@ func Test_viewRun(t *testing.T) { Completed • fix something • OWNER/REPO#101 Started on behalf of octocat about 6 hours ago + For detailed session logs, try: + gh agent-task view 'some-session-id' --log + View this session on GitHub: https://github.com/OWNER/REPO/pull/101/agent-sessions/some-session-id `), @@ -810,6 +858,102 @@ func Test_viewRun(t *testing.T) { 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 log (tty)", + tty: true, + opts: ViewOptions{ + SelectorArg: "some-session-id", + SessionID: "some-session-id", + Log: 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: &api.GitHubUser{ + Login: "octocat", + }, + }, nil + } + m.GetSessionLogsFunc = func(_ context.Context, id string) ([]byte, error) { + assert.Equal(t, "some-session-id", id) + return []byte(""), nil + } + }, + logRendererStubs: func(t *testing.T, m *shared.LogRendererMock) { + m.RenderFunc = func(raw []byte, w io.Writer, cs *iostreams.ColorScheme) (bool, error) { + w.Write([]byte("(rendered:) " + string(raw) + "\n")) + return false, nil + } + }, + wantOut: heredoc.Doc(` + Completed + Started on behalf of octocat about 6 hours ago + + To follow session logs, try: + gh agent-task view 'some-session-id' --log --follow + + (rendered:) + `), + }, + { + name: "with log and follow (tty)", + tty: true, + opts: ViewOptions{ + SelectorArg: "some-session-id", + SessionID: "some-session-id", + Log: true, + Follow: true, + Sleep: func(_ time.Duration) {}, + }, + 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: &api.GitHubUser{ + Login: "octocat", + }, + }, nil + } + + var count int + m.GetSessionLogsFunc = func(_ context.Context, id string) ([]byte, error) { + assert.Equal(t, "some-session-id", id) + + count++ + require.Less(t, count, 3, "too many calls to fetch logs") + if count == 1 { + return []byte(""), nil + } + return []byte(""), nil + } + }, + logRendererStubs: func(t *testing.T, m *shared.LogRendererMock) { + m.FollowFunc = func(fetcher func() ([]byte, error), w io.Writer, cs *iostreams.ColorScheme) error { + raw, err := fetcher() + require.NoError(t, err) + w.Write([]byte("(rendered:) " + string(raw) + "\n")) + + raw, err = fetcher() + require.NoError(t, err) + w.Write([]byte("(rendered:) " + string(raw) + "\n")) + return nil + } + }, + wantOut: heredoc.Doc(` + Completed + Started on behalf of octocat about 6 hours ago + + (rendered:) + (rendered:) + `), + }, } for _, tt := range tests { @@ -824,6 +968,11 @@ func Test_viewRun(t *testing.T) { tt.promptStubs(t, prompter) } + logRenderer := &shared.LogRendererMock{} + if tt.logRendererStubs != nil { + tt.logRendererStubs(t, logRenderer) + } + ios, _, stdout, stderr := iostreams.Test() ios.SetStdoutTTY(tt.tty) @@ -836,6 +985,9 @@ func Test_viewRun(t *testing.T) { opts.CapiClient = func() (capi.CapiClient, error) { return capiClientMock, nil } + opts.LogRenderer = func() shared.LogRenderer { + return logRenderer + } err := viewRun(&opts) if tt.wantErr != nil {