Skip to content

Commit 03f4b77

Browse files
BagToadBagToad
authored
Merge pull request cli#11762 from cli/babakks/remove-prompt-from-agent-task-create
`gh agent-task create`: remove unnecessary prompt and use pager
2 parents 2116327 + 863329b commit 03f4b77

2 files changed

Lines changed: 94 additions & 97 deletions

File tree

‎pkg/cmd/agent-task/create/create.go‎

Lines changed: 26 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,9 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co
7070
// Populate ProblemStatement from arg
7171
if len(args) > 0 {
7272
opts.ProblemStatement = args[0]
73+
if strings.TrimSpace(opts.ProblemStatement) == "" {
74+
return cmdutil.FlagErrorf("task description cannot be empty")
75+
}
7376
} else if opts.ProblemStatementFile == "" && !opts.IO.CanPrompt() {
7477
return cmdutil.FlagErrorf("a task description or -F is required when running non-interactively")
7578
}
@@ -121,36 +124,30 @@ func createRun(opts *CreateOptions) error {
121124
}
122125

123126
if opts.ProblemStatement == "" {
124-
// Load initial problem statement from file, if provided
125127
if opts.ProblemStatementFile != "" {
126128
fileContent, err := cmdutil.ReadFile(opts.ProblemStatementFile, opts.IO.In)
127129
if err != nil {
128-
return cmdutil.FlagErrorf("could not read task description file: %v", err)
130+
return fmt.Errorf("could not read task description file: %w", err)
131+
}
132+
133+
trimmed := strings.TrimSpace(string(fileContent))
134+
if trimmed == "" {
135+
return errors.New("task description file cannot be empty")
129136
}
130-
opts.ProblemStatement = strings.TrimSpace(string(fileContent))
131-
}
132137

133-
if opts.IO.CanPrompt() {
138+
opts.ProblemStatement = trimmed
139+
} else {
134140
desc, err := opts.Prompter.MarkdownEditor("Enter the task description", opts.ProblemStatement, false)
135141
if err != nil {
136142
return err
137143
}
138-
opts.ProblemStatement = strings.TrimSpace(desc)
139-
}
140-
}
141144

142-
if opts.ProblemStatement == "" {
143-
fmt.Fprintf(opts.IO.ErrOut, "a task description is required.\n")
144-
return cmdutil.SilentError
145-
}
145+
trimmed := strings.TrimSpace(string(desc))
146+
if trimmed == "" {
147+
return errors.New("a task description is required")
148+
}
146149

147-
if opts.IO.CanPrompt() {
148-
confirm, err := opts.Prompter.Confirm("Submit agent task", true)
149-
if err != nil {
150-
return err
151-
}
152-
if !confirm {
153-
return cmdutil.SilentError
150+
opts.ProblemStatement = trimmed
154151
}
155152
}
156153

@@ -168,6 +165,10 @@ func createRun(opts *CreateOptions) error {
168165
return err
169166
}
170167

168+
if opts.Follow {
169+
return followLogs(opts, client, job.SessionID)
170+
}
171+
171172
sessionURL, err := fetchJobSessionURL(ctx, client, repo, job, opts.BackOff)
172173
opts.IO.StopProgressIndicator()
173174

@@ -183,9 +184,6 @@ func createRun(opts *CreateOptions) error {
183184
fmt.Fprintf(opts.IO.Out, "job %s queued. View progress: %s\n", job.ID, capi.AgentsHomeURL)
184185
}
185186

186-
if opts.Follow {
187-
return followLogs(opts, client, job.SessionID)
188-
}
189187
return nil
190188
}
191189

@@ -256,8 +254,13 @@ func fetchJobWithBackoff(ctx context.Context, client capi.CapiClient, repo ghrep
256254
}
257255

258256
func followLogs(opts *CreateOptions, capiClient capi.CapiClient, sessionID string) error {
259-
ctx := context.Background()
257+
if err := opts.IO.StartPager(); err == nil {
258+
defer opts.IO.StopPager()
259+
} else {
260+
fmt.Fprintf(opts.IO.ErrOut, "error starting pager: %v\n", err)
261+
}
260262

263+
ctx := context.Background()
261264
renderer := opts.LogRenderer()
262265

263266
var called bool
@@ -273,6 +276,5 @@ func followLogs(opts *CreateOptions, capiClient capi.CapiClient, sessionID strin
273276
return raw, nil
274277
}
275278

276-
fmt.Fprintln(opts.IO.Out, "")
277279
return renderer.Follow(fetcher, opts.IO.Out, opts.IO)
278280
}

‎pkg/cmd/agent-task/create/create_test.go‎

Lines changed: 68 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,21 @@ func TestNewCmdCreate(t *testing.T) {
4646
ProblemStatementFile: "",
4747
},
4848
},
49+
{
50+
name: "empty arg",
51+
args: "''",
52+
wantErr: "task description cannot be empty",
53+
},
54+
{
55+
name: "whitespace arg",
56+
args: "' '",
57+
wantErr: "task description cannot be empty",
58+
},
59+
{
60+
name: "whitespace and newline arg",
61+
args: "'\n'",
62+
wantErr: "task description cannot be empty",
63+
},
4964
{
5065
name: "mutually exclusive arg and file",
5166
args: "'some task inline' -F foo.md",
@@ -96,7 +111,7 @@ func TestNewCmdCreate(t *testing.T) {
96111

97112
_, err = cmd.ExecuteC()
98113
if tt.wantErr != "" {
99-
require.Error(t, err, tt.wantErr)
114+
require.EqualError(t, err, tt.wantErr)
100115
} else {
101116
require.NoError(t, err)
102117
}
@@ -158,72 +173,85 @@ func Test_createRun(t *testing.T) {
158173
wantErrIs error
159174
}{
160175
{
161-
name: "interactive with file prompts to edit with file contents",
176+
name: "interactive, problem statement from arg",
177+
isTTY: true,
178+
opts: &CreateOptions{
179+
BaseRepo: func() (ghrepo.Interface, error) { return ghrepo.New("OWNER", "REPO"), nil },
180+
ProblemStatement: "task description from arg",
181+
},
182+
capiStubs: func(t *testing.T, m *capi.CapiClientMock) {
183+
m.CreateJobFunc = func(ctx context.Context, owner, repo, problemStatement, baseBranch string) (*capi.Job, error) {
184+
require.Equal(t, "OWNER", owner)
185+
require.Equal(t, "REPO", repo)
186+
require.Equal(t, "task description from arg", problemStatement)
187+
return &createdJobSuccessWithPR, nil
188+
}
189+
},
190+
wantStdout: "https://github.com/OWNER/REPO/pull/42/agent-sessions/sess1\n",
191+
},
192+
{
193+
name: "non-interactive, problem statement from arg",
194+
opts: &CreateOptions{
195+
BaseRepo: func() (ghrepo.Interface, error) { return ghrepo.New("OWNER", "REPO"), nil },
196+
ProblemStatement: "task description from arg",
197+
},
198+
capiStubs: func(t *testing.T, m *capi.CapiClientMock) {
199+
m.CreateJobFunc = func(ctx context.Context, owner, repo, problemStatement, baseBranch string) (*capi.Job, error) {
200+
require.Equal(t, "OWNER", owner)
201+
require.Equal(t, "REPO", repo)
202+
require.Equal(t, "task description from arg", problemStatement)
203+
return &createdJobSuccessWithPR, nil
204+
}
205+
},
206+
wantStdout: "https://github.com/OWNER/REPO/pull/42/agent-sessions/sess1\n",
207+
},
208+
{
209+
name: "interactive, problem statement from file",
210+
isTTY: true,
162211
opts: &CreateOptions{
163212
BaseRepo: func() (ghrepo.Interface, error) { return ghrepo.New("OWNER", "REPO"), nil },
164213
ProblemStatement: "",
165214
ProblemStatementFile: taskDescFile,
166-
Prompter: &prompter.PrompterMock{
167-
MarkdownEditorFunc: func(prompt, defaultValue string, blankAllowed bool) (string, error) {
168-
require.Equal(t, "Enter the task description", prompt)
169-
require.Equal(t, "task description from file", defaultValue)
170-
return "edited task description", nil
171-
},
172-
ConfirmFunc: func(message string, defaultValue bool) (bool, error) {
173-
require.Equal(t, "Submit agent task", message)
174-
return true, nil
175-
},
176-
},
177215
},
178-
isTTY: true,
179216
capiStubs: func(t *testing.T, m *capi.CapiClientMock) {
180217
m.CreateJobFunc = func(ctx context.Context, owner, repo, problemStatement, baseBranch string) (*capi.Job, error) {
181218
require.Equal(t, "OWNER", owner)
182219
require.Equal(t, "REPO", repo)
183-
require.Equal(t, "edited task description", problemStatement)
220+
require.Equal(t, "task description from file", problemStatement)
184221
return &createdJobSuccessWithPR, nil
185222
}
186223
},
187224
wantStdout: "https://github.com/OWNER/REPO/pull/42/agent-sessions/sess1\n",
188225
},
189226
{
190-
name: "interactively rejecting confirmation prompt aborts task creation",
227+
name: "non-interactive, problem statement loaded from file",
191228
opts: &CreateOptions{
192-
BaseRepo: func() (ghrepo.Interface, error) { return ghrepo.New("OWNER", "REPO"), nil },
193-
ProblemStatement: "",
194-
Prompter: &prompter.PrompterMock{
195-
MarkdownEditorFunc: func(prompt, defaultValue string, blankAllowed bool) (string, error) {
196-
require.Equal(t, "Enter the task description", prompt)
197-
return "From editor", nil
198-
},
199-
ConfirmFunc: func(message string, defaultValue bool) (bool, error) {
200-
require.Equal(t, "Submit agent task", message)
201-
return false, nil
202-
},
203-
},
229+
BaseRepo: func() (ghrepo.Interface, error) { return ghrepo.New("OWNER", "REPO"), nil },
230+
ProblemStatement: "",
231+
ProblemStatementFile: taskDescFile,
232+
},
233+
capiStubs: func(t *testing.T, m *capi.CapiClientMock) {
234+
m.CreateJobFunc = func(ctx context.Context, owner, repo, problemStatement, baseBranch string) (*capi.Job, error) {
235+
require.Equal(t, "OWNER", owner)
236+
require.Equal(t, "REPO", repo)
237+
require.Equal(t, "task description from file", problemStatement)
238+
return &createdJobSuccessWithPR, nil
239+
}
204240
},
205-
isTTY: true,
206-
wantErr: "SilentError",
207-
wantErrIs: cmdutil.SilentError,
208-
wantStdErr: "",
241+
wantStdout: "https://github.com/OWNER/REPO/pull/42/agent-sessions/sess1\n",
209242
},
210243
{
211-
name: "interactively entering task description with editor, no file",
244+
name: "interactive, problem statement from prompt/editor",
212245
isTTY: true,
213246
opts: &CreateOptions{
214247
BaseRepo: func() (ghrepo.Interface, error) {
215248
return ghrepo.New("OWNER", "REPO"), nil
216249
},
217-
ProblemStatement: "",
218250
Prompter: &prompter.PrompterMock{
219251
MarkdownEditorFunc: func(prompt, defaultValue string, blankAllowed bool) (string, error) {
220252
require.Equal(t, "Enter the task description", prompt)
221253
return "From editor", nil
222254
},
223-
ConfirmFunc: func(message string, defaultValue bool) (bool, error) {
224-
require.Equal(t, "Submit agent task", message)
225-
return true, nil
226-
},
227255
},
228256
},
229257
capiStubs: func(t *testing.T, m *capi.CapiClientMock) {
@@ -235,7 +263,7 @@ func Test_createRun(t *testing.T) {
235263
wantStdout: "https://github.com/OWNER/REPO/pull/42/agent-sessions/sess1\n",
236264
},
237265
{
238-
name: "empty task description from interactive prompt returns error",
266+
name: "interactive, empty task description from editor returns error",
239267
isTTY: true,
240268
opts: &CreateOptions{
241269
BaseRepo: func() (ghrepo.Interface, error) {
@@ -247,26 +275,7 @@ func Test_createRun(t *testing.T) {
247275
},
248276
},
249277
},
250-
wantErr: "SilentError",
251-
wantErrIs: cmdutil.SilentError,
252-
wantStdErr: "a task description is required.\n",
253-
},
254-
{
255-
name: "problem statement loaded from file non-interactively doesn't prompt or return error",
256-
opts: &CreateOptions{
257-
BaseRepo: func() (ghrepo.Interface, error) { return ghrepo.New("OWNER", "REPO"), nil },
258-
ProblemStatement: "",
259-
ProblemStatementFile: taskDescFile,
260-
},
261-
capiStubs: func(t *testing.T, m *capi.CapiClientMock) {
262-
m.CreateJobFunc = func(ctx context.Context, owner, repo, problemStatement, baseBranch string) (*capi.Job, error) {
263-
require.Equal(t, "OWNER", owner)
264-
require.Equal(t, "REPO", repo)
265-
require.Equal(t, "task description from file", problemStatement)
266-
return &createdJobSuccessWithPR, nil
267-
}
268-
},
269-
wantStdout: "https://github.com/OWNER/REPO/pull/42/agent-sessions/sess1\n",
278+
wantErr: "a task description is required",
270279
},
271280
{
272281
name: "missing repo returns error",
@@ -276,18 +285,6 @@ func Test_createRun(t *testing.T) {
276285
}},
277286
wantErr: "a repository is required; re-run in a repository or supply one with --repo owner/name",
278287
},
279-
{
280-
name: "non-interactive empty description returns error",
281-
opts: &CreateOptions{
282-
BaseRepo: func() (ghrepo.Interface, error) {
283-
return ghrepo.New("OWNER", "REPO"), nil
284-
},
285-
ProblemStatement: "",
286-
},
287-
wantErr: "SilentError",
288-
wantErrIs: cmdutil.SilentError,
289-
wantStdErr: "a task description is required.\n",
290-
},
291288
{
292289
name: "problem statement loaded from arg non-interactively doesn't prompt or return error",
293290
opts: &CreateOptions{
@@ -491,8 +488,6 @@ func Test_createRun(t *testing.T) {
491488
}
492489
},
493490
wantStdout: heredoc.Doc(`
494-
https://github.com/OWNER/REPO/pull/42/agent-sessions/sess1
495-
496491
(rendered:) <raw-logs-one>
497492
(rendered:) <raw-logs-two>
498493
`),

0 commit comments

Comments
 (0)