From ea02032f5347a0031d9d4ffeff4bd0978ca5795a Mon Sep 17 00:00:00 2001 From: Sebastian Florek Date: Tue, 28 Jul 2026 14:39:43 +0200 Subject: [PATCH 1/5] feat(workbenches): implement PR follow-up feature - Add `CreateWorkbenchPRFollowup` method in console client mocks. - Implement `pr-followup` command to send follow-up prompts for workbench jobs. - Include `workbenches` command in CLI to manage Plural workbenches. - Rename `ListaStacks` to `ListStacks` in multiple files for consistency. - Update dependencies in `go.mod`. Add tests for workbench features, including PR follow-up client and resolver logic. --- .gitignore | 3 + cmd/command/plural/plural.go | 4 +- cmd/command/stacks/stacks.go | 7 +- .../workbenches/pr_followup_service.go | 80 ++++++ .../workbenches/pr_followup_service_test.go | 135 ++++++++++ .../workbenches/pull_request_provider.go | 83 ++++++ .../workbenches/pull_request_repository.go | 18 ++ .../workbenches/pull_request_resolver.go | 196 ++++++++++++++ .../workbenches/pull_request_resolver_test.go | 239 ++++++++++++++++++ cmd/command/workbenches/workbenches.go | 111 ++++++++ cmd/command/workbenches/workbenches_test.go | 99 ++++++++ go.mod | 4 +- go.sum | 12 +- pkg/console/console.go | 3 +- pkg/console/stacks.go | 2 +- pkg/console/workbenches.go | 22 ++ pkg/test/mocks/ConsoleClient.go | 34 ++- 17 files changed, 1035 insertions(+), 17 deletions(-) create mode 100644 cmd/command/workbenches/pr_followup_service.go create mode 100644 cmd/command/workbenches/pr_followup_service_test.go create mode 100644 cmd/command/workbenches/pull_request_provider.go create mode 100644 cmd/command/workbenches/pull_request_repository.go create mode 100644 cmd/command/workbenches/pull_request_resolver.go create mode 100644 cmd/command/workbenches/pull_request_resolver_test.go create mode 100644 cmd/command/workbenches/workbenches.go create mode 100644 cmd/command/workbenches/workbenches_test.go create mode 100644 pkg/console/workbenches.go diff --git a/.gitignore b/.gitignore index ad8261074..11e1353a8 100644 --- a/.gitignore +++ b/.gitignore @@ -33,3 +33,6 @@ plural*.o # Vendored dependencies vendor/ + +# Agents +.codex/ \ No newline at end of file diff --git a/cmd/command/plural/plural.go b/cmd/command/plural/plural.go index 6980a65b1..15df2fa2d 100644 --- a/cmd/command/plural/plural.go +++ b/cmd/command/plural/plural.go @@ -17,6 +17,7 @@ import ( "github.com/pluralsh/plural-cli/cmd/command/stacks" "github.com/pluralsh/plural-cli/cmd/command/up" "github.com/pluralsh/plural-cli/cmd/command/version" + "github.com/pluralsh/plural-cli/cmd/command/workbenches" "github.com/pluralsh/plural-cli/pkg/client" "github.com/pluralsh/plural-cli/pkg/common" conf "github.com/pluralsh/plural-cli/pkg/config" @@ -103,7 +104,7 @@ func CreateNewApp(plural *Plural) *cli.App { app.Usage = "Tooling to manage your installed plural applications" app.EnableBashCompletion = true app.Flags = globalFlags() - commands := make([]cli.Command, 0, 15+len(plural.getCommands())) + commands := make([]cli.Command, 0, 16+len(plural.getCommands())) commands = append(commands, api.Command(plural.Plural), agentscmd.Command(plural.Plural), @@ -121,6 +122,7 @@ func CreateNewApp(plural *Plural) *cli.App { cmdinit.Command(plural.Plural), up.Command(plural.Plural), version.Command(), + workbenches.Command(plural.Plural), ) commands = append(commands, plural.getCommands()...) app.Commands = commands diff --git a/cmd/command/stacks/stacks.go b/cmd/command/stacks/stacks.go index a5e610b5d..b6dc5dc1d 100644 --- a/cmd/command/stacks/stacks.go +++ b/cmd/command/stacks/stacks.go @@ -4,14 +4,15 @@ import ( "fmt" "github.com/AlecAivazis/survey/v2" + "github.com/samber/lo" + "github.com/urfave/cli" + "github.com/pluralsh/plural-cli/pkg/api" "github.com/pluralsh/plural-cli/pkg/client" "github.com/pluralsh/plural-cli/pkg/common" "github.com/pluralsh/plural-cli/pkg/config" "github.com/pluralsh/plural-cli/pkg/stacks" "github.com/pluralsh/plural-cli/pkg/utils/git" - "github.com/samber/lo" - "github.com/urfave/cli" ) func init() { @@ -83,7 +84,7 @@ func (p *Plural) handleGenerateBackend(_ *cli.Context) error { } stackNames := make(map[string]string) - infrastructureStacks, err := p.ConsoleClient.ListaStacks() + infrastructureStacks, err := p.ConsoleClient.ListStacks() if err != nil { return api.GetErrorResponse(err, "ListaStacks") } diff --git a/cmd/command/workbenches/pr_followup_service.go b/cmd/command/workbenches/pr_followup_service.go new file mode 100644 index 000000000..8ee94acf1 --- /dev/null +++ b/cmd/command/workbenches/pr_followup_service.go @@ -0,0 +1,80 @@ +package workbenches + +import ( + "fmt" + "strings" +) + +const pullRequestNotFoundError = "pull request not found" + +type PRFollowupClient interface { + CreateWorkbenchPRFollowup(url, prompt string) (string, error) +} + +type PullRequestURLResolver interface { + Resolve(options PullRequestOptions) (string, error) +} + +type PRFollowupService struct { + client PRFollowupClient + resolver PullRequestURLResolver +} + +type PRFollowupOptions struct { + Prompt string + PullRequest PullRequestOptions + SkipMissing bool +} + +type PRFollowupResult struct { + ActivityID string + PullRequestURL string + Skipped bool +} + +func NewPRFollowupService(client PRFollowupClient, resolver PullRequestURLResolver) *PRFollowupService { + return &PRFollowupService{client: client, resolver: resolver} +} + +func (s *PRFollowupService) Create(options PRFollowupOptions) (PRFollowupResult, error) { + if strings.TrimSpace(options.Prompt) == "" { + return PRFollowupResult{}, fmt.Errorf("prompt cannot be empty") + } + if s.client == nil { + return PRFollowupResult{}, fmt.Errorf("workbench PR follow-up client is not configured") + } + if s.resolver == nil { + return PRFollowupResult{}, fmt.Errorf("pull request URL resolver is not configured") + } + + pullRequestURL, err := s.resolver.Resolve(options.PullRequest) + if err != nil { + return PRFollowupResult{}, err + } + + activityID, err := s.client.CreateWorkbenchPRFollowup(pullRequestURL, options.Prompt) + if err != nil { + if options.SkipMissing && isPullRequestNotFound(err) { + return PRFollowupResult{PullRequestURL: pullRequestURL, Skipped: true}, nil + } + + return PRFollowupResult{}, err + } + if activityID == "" { + return PRFollowupResult{}, fmt.Errorf("console returned an empty workbench PR follow-up response") + } + + return PRFollowupResult{ActivityID: activityID, PullRequestURL: pullRequestURL}, nil +} + +func isPullRequestNotFound(err error) bool { + if err == nil { + return false + } + + message := err.Error() + return message == pullRequestNotFoundError || + strings.HasPrefix(message, pullRequestNotFoundError+":") || + strings.HasSuffix(message, ": "+pullRequestNotFoundError) || + strings.Contains(message, ": "+pullRequestNotFoundError+":") +} diff --git a/cmd/command/workbenches/pr_followup_service_test.go b/cmd/command/workbenches/pr_followup_service_test.go new file mode 100644 index 000000000..1afbfcb44 --- /dev/null +++ b/cmd/command/workbenches/pr_followup_service_test.go @@ -0,0 +1,135 @@ +package workbenches + +import ( + "errors" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +type fakePRFollowupClient struct { + activityID string + err error + url string + prompt string + calls int +} + +func (f *fakePRFollowupClient) CreateWorkbenchPRFollowup(url, prompt string) (string, error) { + f.url = url + f.prompt = prompt + f.calls++ + return f.activityID, f.err +} + +type fakePullRequestResolver struct { + url string + err error + options PullRequestOptions + calls int +} + +func (f *fakePullRequestResolver) Resolve(options PullRequestOptions) (string, error) { + f.options = options + f.calls++ + return f.url, f.err +} + +func TestPRFollowupServiceUsesResolvedURLAndClientResponse(t *testing.T) { + client := &fakePRFollowupClient{activityID: "activity-1"} + resolver := &fakePullRequestResolver{url: "https://github.com/pluralsh/plural-cli/pull/5078"} + service := NewPRFollowupService(client, resolver) + options := PRFollowupOptions{ + Prompt: " verify the fix ", + PullRequest: PullRequestOptions{Commit: "HEAD~1"}, + } + + result, err := service.Create(options) + + require.NoError(t, err) + assert.Equal(t, PRFollowupResult{ + ActivityID: "activity-1", + PullRequestURL: resolver.url, + }, result) + assert.Equal(t, options.PullRequest, resolver.options) + assert.Equal(t, resolver.url, client.url) + assert.Equal(t, options.Prompt, client.prompt) + assert.Equal(t, 1, client.calls) +} + +func TestPRFollowupServicePropagatesClientError(t *testing.T) { + client := &fakePRFollowupClient{err: errors.New("job is currently active")} + resolver := &fakePullRequestResolver{url: "https://github.com/pluralsh/plural-cli/pull/5078"} + service := NewPRFollowupService(client, resolver) + + _, err := service.Create(PRFollowupOptions{Prompt: "verify the fix"}) + + require.EqualError(t, err, "job is currently active") +} + +func TestPRFollowupServiceSkipsMissingPullRequest(t *testing.T) { + client := &fakePRFollowupClient{err: errors.New("GraphQL error: pull request not found: WorkbenchPrFollowup")} + resolver := &fakePullRequestResolver{url: "https://github.com/pluralsh/plural-cli/pull/5078"} + service := NewPRFollowupService(client, resolver) + + result, err := service.Create(PRFollowupOptions{ + Prompt: "verify the fix", + SkipMissing: true, + }) + + require.NoError(t, err) + assert.Equal(t, PRFollowupResult{ + PullRequestURL: resolver.url, + Skipped: true, + }, result) + assert.Equal(t, 1, client.calls) +} + +func TestPRFollowupServiceDoesNotSkipOtherErrors(t *testing.T) { + client := &fakePRFollowupClient{err: errors.New("GraphQL error: unauthorized")} + resolver := &fakePullRequestResolver{url: "https://github.com/pluralsh/plural-cli/pull/5078"} + service := NewPRFollowupService(client, resolver) + + _, err := service.Create(PRFollowupOptions{ + Prompt: "verify the fix", + SkipMissing: true, + }) + + require.EqualError(t, err, "GraphQL error: unauthorized") +} + +func TestPRFollowupServiceDoesNotSkipUnrelatedErrorContainingMissingText(t *testing.T) { + client := &fakePRFollowupClient{err: errors.New("failed while checking whether pull request not found should be ignored")} + resolver := &fakePullRequestResolver{url: "https://github.com/pluralsh/plural-cli/pull/5078"} + service := NewPRFollowupService(client, resolver) + + _, err := service.Create(PRFollowupOptions{ + Prompt: "verify the fix", + SkipMissing: true, + }) + + require.EqualError(t, err, "failed while checking whether pull request not found should be ignored") +} + +func TestPRFollowupServiceRejectsEmptyClientResponse(t *testing.T) { + client := &fakePRFollowupClient{} + resolver := &fakePullRequestResolver{url: "https://github.com/pluralsh/plural-cli/pull/5078"} + service := NewPRFollowupService(client, resolver) + + _, err := service.Create(PRFollowupOptions{Prompt: "verify the fix"}) + + require.EqualError(t, err, "console returned an empty workbench PR follow-up response") +} + +func TestPRFollowupServiceValidatesPromptBeforeResolvingURL(t *testing.T) { + client := &fakePRFollowupClient{} + resolver := &fakePullRequestResolver{err: errors.New("should not be called")} + service := NewPRFollowupService(client, resolver) + + _, err := service.Create(PRFollowupOptions{Prompt: " \t "}) + + require.EqualError(t, err, "prompt cannot be empty") + assert.Zero(t, resolver.calls) + assert.Zero(t, client.calls) +} diff --git a/cmd/command/workbenches/pull_request_provider.go b/cmd/command/workbenches/pull_request_provider.go new file mode 100644 index 000000000..117610316 --- /dev/null +++ b/cmd/command/workbenches/pull_request_provider.go @@ -0,0 +1,83 @@ +package workbenches + +import ( + "regexp" + "strings" +) + +type ProviderName string + +const ( + ProviderAuto ProviderName = "auto" + ProviderGitHub ProviderName = "github" + ProviderGitLab ProviderName = "gitlab" + ProviderBitbucket ProviderName = "bitbucket" +) + +type PullRequestProvider interface { + Name() ProviderName + Supports(host string) bool + PullRequestNumber(subject string) (string, bool) + PullRequestURL(repositoryURL, number string) string +} + +type pullRequestProvider struct { + name ProviderName + hostMarker string + path string + patterns []*regexp.Regexp +} + +func defaultPullRequestProviders() []PullRequestProvider { + return []PullRequestProvider{ + pullRequestProvider{ + name: ProviderGitHub, + hostMarker: "github", + path: "/pull/", + patterns: []*regexp.Regexp{ + regexp.MustCompile(`\(#([0-9]+)\)`), + regexp.MustCompile(`(?i)merge pull request #([0-9]+)\b`), + }, + }, + pullRequestProvider{ + name: ProviderGitLab, + hostMarker: "gitlab", + path: "/-/merge_requests/", + patterns: []*regexp.Regexp{ + regexp.MustCompile(`!([0-9]+)\b`), + }, + }, + pullRequestProvider{ + name: ProviderBitbucket, + hostMarker: "bitbucket", + path: "/pull-requests/", + patterns: []*regexp.Regexp{ + regexp.MustCompile(`(?i)pull request #([0-9]+)\b`), + }, + }, + } +} + +func (p pullRequestProvider) Name() ProviderName { + return p.name +} + +func (p pullRequestProvider) Supports(host string) bool { + return strings.Contains(strings.ToLower(host), p.hostMarker) +} + +func (p pullRequestProvider) PullRequestNumber(subject string) (string, bool) { + for _, pattern := range p.patterns { + matches := pattern.FindStringSubmatch(subject) + + if len(matches) == 2 { + return matches[1], true + } + } + + return "", false +} + +func (p pullRequestProvider) PullRequestURL(repositoryURL, number string) string { + return repositoryURL + p.path + number +} diff --git a/cmd/command/workbenches/pull_request_repository.go b/cmd/command/workbenches/pull_request_repository.go new file mode 100644 index 000000000..e84bcac19 --- /dev/null +++ b/cmd/command/workbenches/pull_request_repository.go @@ -0,0 +1,18 @@ +package workbenches + +import gitutils "github.com/pluralsh/plural-cli/pkg/utils/git" + +type PullRequestRepository interface { + CommitSubject(ref string) (string, error) + RemoteURL() (string, error) +} + +type GitPullRequestRepository struct{} + +func (GitPullRequestRepository) CommitSubject(ref string) (string, error) { + return gitutils.GitRaw("show", "-s", "--format=%s", ref) +} + +func (GitPullRequestRepository) RemoteURL() (string, error) { + return gitutils.GitRaw("remote", "get-url", "origin") +} diff --git a/cmd/command/workbenches/pull_request_resolver.go b/cmd/command/workbenches/pull_request_resolver.go new file mode 100644 index 000000000..c93338076 --- /dev/null +++ b/cmd/command/workbenches/pull_request_resolver.go @@ -0,0 +1,196 @@ +package workbenches + +import ( + "fmt" + "net/url" + "strings" + + "github.com/samber/lo" +) + +type PullRequestOptions struct { + URL string + Commit string + BaseURL string + Provider string +} + +type PullRequestResolver struct { + repository PullRequestRepository + providers []PullRequestProvider +} + +type repositoryAddress struct { + url string + host string +} + +func NewPullRequestResolver(repository PullRequestRepository) *PullRequestResolver { + if repository == nil { + repository = GitPullRequestRepository{} + } + + return &PullRequestResolver{ + repository: repository, + providers: defaultPullRequestProviders(), + } +} + +func (r *PullRequestResolver) Resolve(options PullRequestOptions) (string, error) { + if options.URL != "" && options.Commit != "" { + return "", fmt.Errorf("url and commit cannot be used together") + } + + if options.URL != "" { + if err := r.validateHTTPURL(options.URL); err != nil { + return "", fmt.Errorf("invalid pull request URL: %w", err) + } + + return options.URL, nil + } + + if r.repository == nil { + return "", fmt.Errorf("git repository is not configured") + } + + var provider PullRequestProvider + var err error + if options.Provider != "" && options.Provider != string(ProviderAuto) { + provider, err = r.provider(options.Provider, "") + + if err != nil { + return "", err + } + } + + ref := options.Commit + if ref == "" { + ref = "HEAD" + } + + subject, err := r.repository.CommitSubject(ref) + if err != nil { + return "", fmt.Errorf("could not read commit %q: %w", ref, err) + } + + if options.BaseURL != "" { + if err := r.validateHTTPURL(options.BaseURL); err != nil { + return "", fmt.Errorf("invalid repository base URL: %w", err) + } + } + + address := repositoryAddress{url: options.BaseURL} + if options.Provider == "" || options.Provider == string(ProviderAuto) || address.url == "" { + address, err = r.repositoryAddress(options.BaseURL) + if err != nil { + return "", err + } + } + + if provider == nil { + provider, err = r.provider(options.Provider, address.host) + if err != nil { + return "", err + } + } + + number, found := provider.PullRequestNumber(subject) + if !found { + return "", fmt.Errorf("commit %q does not identify a %s pull request", ref, provider.Name()) + } + + return provider.PullRequestURL(strings.TrimRight(address.url, "/"), number), nil +} + +func (r *PullRequestResolver) provider(name, host string) (PullRequestProvider, error) { + if name == "" || name == string(ProviderAuto) { + provider, found := lo.Find(r.providers, func(provider PullRequestProvider) bool { + return provider.Supports(host) + }) + + if !found { + return nil, fmt.Errorf("cannot infer source control provider from host %q; provide --provider", host) + } + + return provider, nil + } + + provider, found := lo.Find(r.providers, func(provider PullRequestProvider) bool { + return string(provider.Name()) == name + }) + + if !found { + return nil, fmt.Errorf("unsupported source control provider %q", name) + } + + return provider, nil +} + +func (r *PullRequestResolver) repositoryAddress(baseURL string) (repositoryAddress, error) { + remote, err := r.repository.RemoteURL() + if err != nil { + return repositoryAddress{}, fmt.Errorf("could not read origin URL: %w", err) + } + + address, err := r.parseRepositoryAddress(remote) + if err != nil { + return repositoryAddress{}, err + } + + if baseURL != "" { + address.url = baseURL + } + + return address, nil +} + +func (*PullRequestResolver) parseRepositoryAddress(raw string) (repositoryAddress, error) { + if raw == "" { + return repositoryAddress{}, fmt.Errorf("origin URL is empty") + } + + if !strings.Contains(raw, "://") { + if at := strings.LastIndex(raw, "@"); at >= 0 { + raw = raw[at+1:] + } + + parts := strings.SplitN(raw, ":", 2) + if len(parts) != 2 || parts[0] == "" || parts[1] == "" { + return repositoryAddress{}, fmt.Errorf("cannot parse origin URL %q", raw) + } + + host := parts[0] + path := strings.TrimSuffix(strings.Trim(parts[1], "/"), ".git") + return repositoryAddress{url: "https://" + host + "/" + path, host: strings.ToLower(host)}, nil + } + + parsed, err := url.Parse(raw) + if err != nil || parsed.Hostname() == "" { + return repositoryAddress{}, fmt.Errorf("cannot parse origin URL %q", raw) + } + + path := strings.TrimSuffix(strings.Trim(parsed.Path, "/"), ".git") + if path == "" { + return repositoryAddress{}, fmt.Errorf("origin URL %q does not contain a repository path", raw) + } + + return repositoryAddress{ + url: "https://" + parsed.Host + "/" + path, + host: strings.ToLower(parsed.Hostname()), + }, nil +} + +func (*PullRequestResolver) validateHTTPURL(raw string) error { + parsed, err := url.Parse(raw) + if err != nil { + return err + } + if parsed.Scheme != "https" && parsed.Scheme != "http" { + return fmt.Errorf("scheme must be http or https") + } + if parsed.Hostname() == "" { + return fmt.Errorf("host is required") + } + + return nil +} diff --git a/cmd/command/workbenches/pull_request_resolver_test.go b/cmd/command/workbenches/pull_request_resolver_test.go new file mode 100644 index 000000000..7e0ca4b6f --- /dev/null +++ b/cmd/command/workbenches/pull_request_resolver_test.go @@ -0,0 +1,239 @@ +package workbenches + +import ( + "errors" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +type fakePullRequestRepository struct { + subject string + remote string + subjectErr error + remoteErr error + requestedRef string + subjectCalls int + remoteURLCalls int +} + +func (f *fakePullRequestRepository) CommitSubject(ref string) (string, error) { + f.requestedRef = ref + f.subjectCalls++ + return f.subject, f.subjectErr +} + +func (f *fakePullRequestRepository) RemoteURL() (string, error) { + f.remoteURLCalls++ + return f.remote, f.remoteErr +} + +func TestPullRequestResolverUsesExplicitURL(t *testing.T) { + repository := &fakePullRequestRepository{subjectErr: errors.New("should not be called")} + resolver := NewPullRequestResolver(repository) + + result, err := resolver.Resolve(PullRequestOptions{URL: "https://github.com/pluralsh/console/pull/3905"}) + + require.NoError(t, err) + assert.Equal(t, "https://github.com/pluralsh/console/pull/3905", result) + assert.Zero(t, repository.subjectCalls) + assert.Zero(t, repository.remoteURLCalls) +} + +func TestPullRequestResolverDoesNotTrimExplicitURL(t *testing.T) { + resolver := NewPullRequestResolver(&fakePullRequestRepository{}) + + _, err := resolver.Resolve(PullRequestOptions{URL: " https://github.com/pluralsh/console/pull/3905 "}) + + require.ErrorContains(t, err, "invalid pull request URL") +} + +func TestPullRequestResolverRejectsURLAndCommit(t *testing.T) { + resolver := NewPullRequestResolver(&fakePullRequestRepository{}) + + _, err := resolver.Resolve(PullRequestOptions{ + URL: "https://github.com/pluralsh/console/pull/3905", + Commit: "HEAD~1", + }) + + require.EqualError(t, err, "url and commit cannot be used together") +} + +func TestPullRequestResolverInfersProviderURL(t *testing.T) { + tests := []struct { + name string + subject string + remote string + expected string + }{ + { + name: "GitHub squash commit over SSH", + subject: "Implement verification (#5078)", + remote: "git@github.com:pluralsh/plural-cli.git", + expected: "https://github.com/pluralsh/plural-cli/pull/5078", + }, + { + name: "GitHub merge commit over HTTPS", + subject: "Merge pull request #3905 from pluralsh/more-perf-improvements", + remote: "https://github.com/pluralsh/console.git", + expected: "https://github.com/pluralsh/console/pull/3905", + }, + { + name: "GitLab merge request over SSH URL", + subject: "See merge request group/project!73", + remote: "ssh://git@gitlab.com/group/project.git", + expected: "https://gitlab.com/group/project/-/merge_requests/73", + }, + { + name: "Bitbucket pull request", + subject: "Merge pull request #19", + remote: "https://bitbucket.org/team/project.git", + expected: "https://bitbucket.org/team/project/pull-requests/19", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + repository := &fakePullRequestRepository{subject: tt.subject, remote: tt.remote} + resolver := NewPullRequestResolver(repository) + + result, err := resolver.Resolve(PullRequestOptions{}) + + require.NoError(t, err) + assert.Equal(t, tt.expected, result) + assert.Equal(t, "HEAD", repository.requestedRef) + assert.Equal(t, 1, repository.remoteURLCalls) + }) + } +} + +func TestPullRequestResolverUsesExplicitProviderAndBaseURL(t *testing.T) { + repository := &fakePullRequestRepository{subject: "See merge request group/project!81"} + resolver := NewPullRequestResolver(repository) + + result, err := resolver.Resolve(PullRequestOptions{ + Commit: "HEAD~1", + BaseURL: "https://code.example.com/team/project", + Provider: "gitlab", + }) + + require.NoError(t, err) + assert.Equal(t, "https://code.example.com/team/project/-/merge_requests/81", result) + assert.Zero(t, repository.remoteURLCalls) +} + +func TestPullRequestResolverUsesExplicitProviderWithSelfHostedOrigin(t *testing.T) { + repository := &fakePullRequestRepository{ + subject: "See merge request group/project!82", + remote: "git@code.example.com:group/project.git", + } + resolver := NewPullRequestResolver(repository) + + result, err := resolver.Resolve(PullRequestOptions{Provider: "gitlab"}) + + require.NoError(t, err) + assert.Equal(t, "https://code.example.com/group/project/-/merge_requests/82", result) + assert.Equal(t, 1, repository.remoteURLCalls) +} + +func TestPullRequestResolverUsesProviderSpecificCommitPattern(t *testing.T) { + repository := &fakePullRequestRepository{ + subject: "GitHub-style title (#82)", + remote: "git@code.example.com:group/project.git", + } + resolver := NewPullRequestResolver(repository) + + _, err := resolver.Resolve(PullRequestOptions{Provider: "gitlab"}) + + require.EqualError(t, err, `commit "HEAD" does not identify a gitlab pull request`) +} + +func TestPullRequestResolverRejectsUnsupportedProvider(t *testing.T) { + resolver := NewPullRequestResolver(&fakePullRequestRepository{}) + + _, err := resolver.Resolve(PullRequestOptions{Provider: "azure-devops"}) + + require.EqualError(t, err, `unsupported source control provider "azure-devops"`) +} + +func TestPullRequestResolverErrors(t *testing.T) { + tests := []struct { + name string + subject string + remote string + subjectErr error + remoteErr error + errorText string + }{ + { + name: "commit lookup fails", + subjectErr: errors.New("unknown revision"), + errorText: `could not read commit "HEAD": unknown revision`, + }, + { + name: "subject has no pull request", + subject: "ordinary commit", + remote: "git@github.com:team/project.git", + errorText: `commit "HEAD" does not identify a github pull request`, + }, + { + name: "remote lookup fails", + subject: "Fix issue (#12)", + remoteErr: errors.New("origin missing"), + errorText: "could not read origin URL: origin missing", + }, + { + name: "unknown provider", + subject: "Fix issue (#12)", + remote: "git@code.example.com:team/project.git", + errorText: `cannot infer source control provider from host "code.example.com"; provide --provider`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + repository := &fakePullRequestRepository{ + subject: tt.subject, + remote: tt.remote, + subjectErr: tt.subjectErr, + remoteErr: tt.remoteErr, + } + resolver := NewPullRequestResolver(repository) + + _, err := resolver.Resolve(PullRequestOptions{}) + + require.EqualError(t, err, tt.errorText) + }) + } +} + +func TestPullRequestResolverParsesRepositoryAddress(t *testing.T) { + tests := []struct { + raw string + expected repositoryAddress + }{ + { + raw: "git@github.com:pluralsh/plural-cli.git", + expected: repositoryAddress{url: "https://github.com/pluralsh/plural-cli", host: "github.com"}, + }, + { + raw: "ssh://git@gitlab.example.com/group/project.git", + expected: repositoryAddress{url: "https://gitlab.example.com/group/project", host: "gitlab.example.com"}, + }, + { + raw: "https://bitbucket.org/team/project.git/", + expected: repositoryAddress{url: "https://bitbucket.org/team/project", host: "bitbucket.org"}, + }, + } + resolver := NewPullRequestResolver(&fakePullRequestRepository{}) + + for _, tt := range tests { + t.Run(tt.raw, func(t *testing.T) { + address, err := resolver.parseRepositoryAddress(tt.raw) + + require.NoError(t, err) + assert.Equal(t, tt.expected, address) + }) + } +} diff --git a/cmd/command/workbenches/workbenches.go b/cmd/command/workbenches/workbenches.go new file mode 100644 index 000000000..cbbbbabb6 --- /dev/null +++ b/cmd/command/workbenches/workbenches.go @@ -0,0 +1,111 @@ +package workbenches + +import ( + "github.com/urfave/cli" + + pluralclient "github.com/pluralsh/plural-cli/pkg/client" + "github.com/pluralsh/plural-cli/pkg/common" + "github.com/pluralsh/plural-cli/pkg/utils" +) + +type Workbenches struct { + pluralclient.Plural + consoleToken string + consoleURL string +} + +func NewWorkbenches(clients pluralclient.Plural) *Workbenches { + return &Workbenches{Plural: clients} +} + +func Command(clients pluralclient.Plural) cli.Command { + return NewWorkbenches(clients).Command() +} + +func (w *Workbenches) Command() cli.Command { + return cli.Command{ + Name: "workbenches", + Aliases: []string{"wb"}, + Usage: "manage Plural workbenches", + Category: "AI", + Flags: []cli.Flag{ + cli.StringFlag{ + Name: "token", + Usage: "console token", + EnvVar: "PLURAL_CONSOLE_TOKEN", + Destination: &w.consoleToken, + }, + cli.StringFlag{ + Name: "console-url", + Usage: "console URL address", + EnvVar: "PLURAL_CONSOLE_URL", + Destination: &w.consoleURL, + }, + }, + Subcommands: []cli.Command{w.prFollowupCommand()}, + } +} + +func (w *Workbenches) prFollowupCommand() cli.Command { + return cli.Command{ + Name: "pr-followup", + Usage: "send a follow-up prompt to the workbench job associated with a pull request", + Action: common.LatestVersion(w.handlePRFollowup), + Flags: []cli.Flag{ + cli.StringFlag{ + Name: "prompt", + Usage: "follow-up prompt", + Required: true, + }, + cli.StringFlag{ + Name: "url", + Usage: "pull request URL; bypasses commit inference", + }, + cli.StringFlag{ + Name: "commit", + Usage: "commit or ref whose subject identifies the pull request (defaults to HEAD)", + }, + cli.StringFlag{ + Name: "base-url", + Usage: "repository web URL used to construct the pull request URL", + }, + cli.StringFlag{ + Name: "provider", + Usage: "source control provider (auto, github, gitlab, or bitbucket)", + Value: string(ProviderAuto), + }, + cli.BoolFlag{ + Name: "skip-missing", + Usage: "exit successfully when the pull request is not associated with a workbench job", + }, + }, + } +} + +func (w *Workbenches) handlePRFollowup(ctx *cli.Context) error { + if err := w.InitConsoleClient(w.consoleToken, w.consoleURL); err != nil { + return err + } + + service := NewPRFollowupService(w.ConsoleClient, NewPullRequestResolver(nil)) + result, err := service.Create(PRFollowupOptions{ + Prompt: ctx.String("prompt"), + SkipMissing: ctx.Bool("skip-missing"), + PullRequest: PullRequestOptions{ + URL: ctx.String("url"), + Commit: ctx.String("commit"), + BaseURL: ctx.String("base-url"), + Provider: ctx.String("provider"), + }, + }) + if err != nil { + return err + } + if result.Skipped { + utils.Success("No workbench job found for %s; skipping\n", result.PullRequestURL) + return nil + } + + utils.Success("Created workbench PR follow-up %s for %s\n", result.ActivityID, result.PullRequestURL) + return nil +} diff --git a/cmd/command/workbenches/workbenches_test.go b/cmd/command/workbenches/workbenches_test.go new file mode 100644 index 000000000..2d15f73b5 --- /dev/null +++ b/cmd/command/workbenches/workbenches_test.go @@ -0,0 +1,99 @@ +package workbenches + +import ( + "errors" + "flag" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" + "github.com/urfave/cli" + + pluralclient "github.com/pluralsh/plural-cli/pkg/client" + "github.com/pluralsh/plural-cli/pkg/test/mocks" +) + +func TestCommandShape(t *testing.T) { + command := Command(pluralclient.Plural{}) + + assert.Equal(t, "workbenches", command.Name) + assert.Contains(t, command.Aliases, "wb") + require.Len(t, command.Subcommands, 1) + assert.Equal(t, "pr-followup", command.Subcommands[0].Name) + assert.Equal(t, map[string]bool{ + "console-url": true, + "token": true, + }, flagNames(command.Flags)) + assert.Equal(t, map[string]bool{ + "base-url": true, + "commit": true, + "prompt": true, + "provider": true, + "skip-missing": true, + "url": true, + }, flagNames(command.Subcommands[0].Flags)) +} + +func TestHandlePRFollowupUsesConfiguredConsoleClient(t *testing.T) { + url := "https://github.com/pluralsh/plural-cli/pull/5078" + prompt := " verify the fix " + consoleMock := mocks.NewConsoleClient(t) + consoleMock.On("CreateWorkbenchPRFollowup", url, prompt).Return("activity-1", nil).Once() + workbenches := NewWorkbenches(pluralclient.Plural{ConsoleClient: consoleMock}) + ctx := prFollowupContext(t, "--url", url, "--prompt", prompt) + + err := workbenches.handlePRFollowup(ctx) + + require.NoError(t, err) + consoleMock.AssertExpectations(t) +} + +func TestHandlePRFollowupPropagatesConsoleError(t *testing.T) { + url := "https://github.com/pluralsh/plural-cli/pull/5078" + consoleMock := mocks.NewConsoleClient(t) + consoleMock.On("CreateWorkbenchPRFollowup", url, mock.Anything).Return("", errors.New("pull request not found")).Once() + workbenches := NewWorkbenches(pluralclient.Plural{ConsoleClient: consoleMock}) + ctx := prFollowupContext(t, "--url", url, "--prompt", "verify") + + err := workbenches.handlePRFollowup(ctx) + + require.EqualError(t, err, "pull request not found") +} + +func TestHandlePRFollowupSkipsMissingPullRequest(t *testing.T) { + url := "https://github.com/pluralsh/plural-cli/pull/5078" + consoleMock := mocks.NewConsoleClient(t) + consoleMock.On("CreateWorkbenchPRFollowup", url, mock.Anything).Return("", errors.New("GraphQL error: pull request not found: WorkbenchPrFollowup")).Once() + workbenches := NewWorkbenches(pluralclient.Plural{ConsoleClient: consoleMock}) + ctx := prFollowupContext(t, "--url", url, "--prompt", "verify", "--skip-missing") + + err := workbenches.handlePRFollowup(ctx) + + require.NoError(t, err) + consoleMock.AssertExpectations(t) +} + +func flagNames(flags []cli.Flag) map[string]bool { + names := make(map[string]bool, len(flags)) + for _, commandFlag := range flags { + names[commandFlag.GetName()] = true + } + + return names +} + +func prFollowupContext(t *testing.T, args ...string) *cli.Context { + t.Helper() + + flags := flag.NewFlagSet("pr-followup", flag.ContinueOnError) + flags.String("url", "", "") + flags.String("commit", "", "") + flags.String("base-url", "", "") + flags.String("prompt", "", "") + flags.String("provider", string(ProviderAuto), "") + flags.Bool("skip-missing", false, "") + require.NoError(t, flags.Parse(args)) + + return cli.NewContext(nil, flags, nil) +} diff --git a/go.mod b/go.mod index 8b1a6b4e1..5f81e5dcf 100644 --- a/go.mod +++ b/go.mod @@ -41,7 +41,7 @@ require ( github.com/mitchellh/mapstructure v1.5.1-0.20231216201459-8508981c8b6c github.com/olekukonko/tablewriter v1.1.4 github.com/pkg/browser v0.0.0-20240102092130-5ac0b6a4141c - github.com/pluralsh/console/go/client v1.76.5 + github.com/pluralsh/console/go/client v1.78.0 github.com/pluralsh/console/go/polly v1.0.0 github.com/pluralsh/gqlclient v1.12.2 github.com/pluralsh/plural-operator v0.6.0 @@ -247,7 +247,7 @@ require ( github.com/Masterminds/goutils v1.1.1 // indirect github.com/Masterminds/semver/v3 v3.5.0 github.com/Masterminds/squirrel v1.5.4 // indirect - github.com/Microsoft/go-winio v0.6.2 // indirect + github.com/Microsoft/go-winio v0.6.3-0.20251027160822-ad3df93bed29 // indirect github.com/ProtonMail/go-crypto v1.4.1 // indirect github.com/asaskevich/govalidator v0.0.0-20230301143203-a9d515a09cc2 // indirect github.com/aws/aws-sdk-go-v2/config v1.32.28 diff --git a/go.sum b/go.sum index 6304309b5..b50a9c329 100644 --- a/go.sum +++ b/go.sum @@ -152,8 +152,8 @@ github.com/Masterminds/squirrel v1.5.4 h1:uUcX/aBc8O7Fg9kaISIUsHXdKuqehiXAMQTYX8 github.com/Masterminds/squirrel v1.5.4/go.mod h1:NNaOrjSoIDfDA40n7sr2tPNZRfjzjA400rg+riTZj10= github.com/Microsoft/go-winio v0.5.0/go.mod h1:JPGBdM1cNvN/6ISo+n8V5iA4v8pBzdOpzfwIujj1a84= github.com/Microsoft/go-winio v0.5.2/go.mod h1:WpS1mjBmmwHBEWmogvA2mj8546UReBk4v8QkMxJ6pZY= -github.com/Microsoft/go-winio v0.6.2 h1:F2VQgta7ecxGYO8k3ZZz3RS8fVIXVxONVUPlNERoyfY= -github.com/Microsoft/go-winio v0.6.2/go.mod h1:yd8OoFMLzJbo9gZq8j5qaps8bJ9aShtEA8Ipt1oGCvU= +github.com/Microsoft/go-winio v0.6.3-0.20251027160822-ad3df93bed29 h1:0kQAzHq8vLs7Pptv+7TxjdETLf/nIqJpIB4oC6Ba4vY= +github.com/Microsoft/go-winio v0.6.3-0.20251027160822-ad3df93bed29/go.mod h1:ZWa7ssZJT30CCDGJ7fk/2SBTq9BIQrrVjrcss0UW2s0= github.com/Netflix/go-expect v0.0.0-20220104043353-73e0943537d2 h1:+vx7roKuyA63nhn5WAunQHLTznkw5W8b1Xc0dNjp83s= github.com/Netflix/go-expect v0.0.0-20220104043353-73e0943537d2/go.mod h1:HBCaDeC1lPdgDeDbhX8XFpy1jqjK0IBG8W5K+xYqA0w= github.com/ProtonMail/go-crypto v1.4.1 h1:9RfcZHqEQUvP8RzecWEUafnZVtEvrBVL9BiF67IQOfM= @@ -608,8 +608,8 @@ github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4= github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/planetscale/vtprotobuf v0.6.1-0.20240319094008-0393e58bdf10 h1:GFCKgmp0tecUJ0sJuv4pzYCqS9+RGSn52M3FUwPs+uo= github.com/planetscale/vtprotobuf v0.6.1-0.20240319094008-0393e58bdf10/go.mod h1:t/avpk3KcrXxUnYOhZhMXJlSEyie6gQbtLq5NM3loB8= -github.com/pluralsh/console/go/client v1.76.5 h1:0DjfQTphB3pDLXVdK4Ty9/EsJvrm8kC12pqB9unvePM= -github.com/pluralsh/console/go/client v1.76.5/go.mod h1:+dJuFu2ruNb8u4Aaor8NPneOiK2ndrwGBsbsFxbQfDU= +github.com/pluralsh/console/go/client v1.78.0 h1:gFvXniJOzG0XZpmPAiMCULvzRs8+yN/HpWwoYhymWIg= +github.com/pluralsh/console/go/client v1.78.0/go.mod h1:COAf7KBKIkB9TJfoP25OZU2RWanwHi8Rvzr3/sl1eH0= github.com/pluralsh/console/go/controller v0.0.0-20260706120905-5f6ff115eb71 h1:d00SB5O8he1ls7YmUpI10J9FiqXKU+k8EgI0gKqRdSo= github.com/pluralsh/console/go/controller v0.0.0-20260706120905-5f6ff115eb71/go.mod h1:7eM3lngWsj4OUrQsuN2ZTT5UgV7c5yi+PZF8Cer3K+M= github.com/pluralsh/console/go/polly v1.0.0 h1:NRg8CMITGllEJ08oYidNuuG/gJGDdDuga8TM+gdIcFA= @@ -653,8 +653,8 @@ github.com/richardartoul/molecule v1.0.1-0.20240531184615-7ca0df43c0b3 h1:4+LEVO github.com/richardartoul/molecule v1.0.1-0.20240531184615-7ca0df43c0b3/go.mod h1:vl5+MqJ1nBINuSsUI2mGgH79UweUT/B5Fy8857PqyyI= github.com/rivo/uniseg v0.4.7 h1:WUdvkW8uEhrYfLC4ZzdpI2ztxP1I582+49Oc5Mq64VQ= github.com/rivo/uniseg v0.4.7/go.mod h1:FN3SvrM+Zdj16jyLfmOkMNblXMcoc8DfTHruCPUcx88= -github.com/rogpeppe/go-internal v1.14.1 h1:UQB4HGPB6osV0SQTLymcB4TgvyWu6ZyliaW0tI/otEQ= -github.com/rogpeppe/go-internal v1.14.1/go.mod h1:MaRKkUm5W0goXpeCfT7UZI6fk/L7L7so1lCWt35ZSgc= +github.com/rogpeppe/go-internal v1.15.0 h1:D0RCU5rMAp+SpgkiNdrjfJ+LX4J1M32V2NeCY7EJ6hc= +github.com/rogpeppe/go-internal v1.15.0/go.mod h1:DrUVZyrJU+txYW5/1kwtXQSMFio52ZOxX7yM1VHvnxs= github.com/rubenv/sql-migrate v1.8.1 h1:EPNwCvjAowHI3TnZ+4fQu3a915OpnQoPAjTXCGOy2U0= github.com/rubenv/sql-migrate v1.8.1/go.mod h1:BTIKBORjzyxZDS6dzoiw6eAFYJ1iNlGAtjn4LGeVjS8= github.com/russross/blackfriday/v2 v2.1.0 h1:JIOH55/0cWyOuilr9/qlrm0BSXldqnqwMsf35Ld67mk= diff --git a/pkg/console/console.go b/pkg/console/console.go index 7a122219c..9fa3a5b84 100644 --- a/pkg/console/console.go +++ b/pkg/console/console.go @@ -60,12 +60,13 @@ type ConsoleClient interface { ListAgentRuns(first int64) ([]*consoleclient.AgentRunMinimalFragment, error) ListStackRuns(stackID string) (*consoleclient.ListStackRuns, error) CreatePullRequest(id string, branch, context *string) (*consoleclient.PullRequestFragment, error) + CreateWorkbenchPRFollowup(url, prompt string) (string, error) GetPrAutomationByName(name string) (*consoleclient.PrAutomationFragment, error) CreateBootstrapToken(attributes consoleclient.BootstrapTokenAttributes) (string, error) CreateClusterRegistration(attributes consoleclient.ClusterRegistrationCreateAttributes) (*consoleclient.ClusterRegistrationFragment, error) IsClusterRegistrationComplete(machineID string) (bool, *consoleclient.ClusterRegistrationFragment) GetUser(email string) (*consoleclient.UserFragment, error) - ListaStacks() (*consoleclient.ListInfrastructureStacks, error) + ListStacks() (*consoleclient.ListInfrastructureStacks, error) } type authedTransport struct { diff --git a/pkg/console/stacks.go b/pkg/console/stacks.go index 3df590e63..15ab55948 100644 --- a/pkg/console/stacks.go +++ b/pkg/console/stacks.go @@ -9,6 +9,6 @@ func (c *consoleClient) ListStackRuns(stackID string) (*gqlclient.ListStackRuns, return c.client.ListStackRuns(c.ctx, stackID, nil, nil, lo.ToPtr(int64(100)), nil) } -func (c *consoleClient) ListaStacks() (*gqlclient.ListInfrastructureStacks, error) { +func (c *consoleClient) ListStacks() (*gqlclient.ListInfrastructureStacks, error) { return c.client.ListInfrastructureStacks(c.ctx, nil, lo.ToPtr(int64(100)), nil, nil) } diff --git a/pkg/console/workbenches.go b/pkg/console/workbenches.go new file mode 100644 index 000000000..375a02b9a --- /dev/null +++ b/pkg/console/workbenches.go @@ -0,0 +1,22 @@ +package console + +import ( + "fmt" + + consoleclient "github.com/pluralsh/console/go/client" + + "github.com/pluralsh/plural-cli/pkg/api" +) + +func (c *consoleClient) CreateWorkbenchPRFollowup(url, prompt string) (string, error) { + result, err := c.client.WorkbenchPrFollowup(c.ctx, url, consoleclient.WorkbenchMessageAttributes{Prompt: prompt}) + if err != nil { + return "", api.GetErrorResponse(err, "WorkbenchPrFollowup") + } + activity := result.GetWorkbenchPrFollowup() + if activity == nil { + return "", fmt.Errorf("returned object [WorkbenchPrFollowup] is nil") + } + + return activity.GetID(), nil +} diff --git a/pkg/test/mocks/ConsoleClient.go b/pkg/test/mocks/ConsoleClient.go index 3ba2c578b..bd440a25a 100644 --- a/pkg/test/mocks/ConsoleClient.go +++ b/pkg/test/mocks/ConsoleClient.go @@ -369,6 +369,34 @@ func (_m *ConsoleClient) CreateRepository(url string, privateKey *string, passph return r0, r1 } +// CreateWorkbenchPRFollowup provides a mock function with given fields: url, prompt +func (_m *ConsoleClient) CreateWorkbenchPRFollowup(url string, prompt string) (string, error) { + ret := _m.Called(url, prompt) + + if len(ret) == 0 { + panic("no return value specified for CreateWorkbenchPRFollowup") + } + + var r0 string + var r1 error + if rf, ok := ret.Get(0).(func(string, string) (string, error)); ok { + return rf(url, prompt) + } + if rf, ok := ret.Get(0).(func(string, string) string); ok { + r0 = rf(url, prompt) + } else { + r0 = ret.Get(0).(string) + } + + if rf, ok := ret.Get(1).(func(string, string) error); ok { + r1 = rf(url, prompt) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + // DeleteCluster provides a mock function with given fields: id func (_m *ConsoleClient) DeleteCluster(id string) error { ret := _m.Called(id) @@ -1111,12 +1139,12 @@ func (_m *ConsoleClient) ListStackRuns(stackID string) (*client.ListStackRuns, e return r0, r1 } -// ListaStacks provides a mock function with no fields -func (_m *ConsoleClient) ListaStacks() (*client.ListInfrastructureStacks, error) { +// ListStacks provides a mock function with no fields +func (_m *ConsoleClient) ListStacks() (*client.ListInfrastructureStacks, error) { ret := _m.Called() if len(ret) == 0 { - panic("no return value specified for ListaStacks") + panic("no return value specified for ListStacks") } var r0 *client.ListInfrastructureStacks From 20050a0d7373315e369baa3eca83567686dcddc9 Mon Sep 17 00:00:00 2001 From: Sebastian Florek Date: Tue, 28 Jul 2026 15:34:09 +0200 Subject: [PATCH 2/5] fix: correct API error response parameter and update workflow CLI installation - Fix typo in `ListStacks` API error response parameter in `stacks.go` - Update Plural CLI installation command with specific commit hash in GitHub Actions workflow --- cmd/command/stacks/stacks.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/command/stacks/stacks.go b/cmd/command/stacks/stacks.go index b6dc5dc1d..f0d3465f9 100644 --- a/cmd/command/stacks/stacks.go +++ b/cmd/command/stacks/stacks.go @@ -86,7 +86,7 @@ func (p *Plural) handleGenerateBackend(_ *cli.Context) error { stackNames := make(map[string]string) infrastructureStacks, err := p.ConsoleClient.ListStacks() if err != nil { - return api.GetErrorResponse(err, "ListaStacks") + return api.GetErrorResponse(err, "ListStacks") } if infrastructureStacks == nil || infrastructureStacks.InfrastructureStacks == nil || len(infrastructureStacks.InfrastructureStacks.Edges) == 0 { return fmt.Errorf("returned objects list [ListStacks] is nil") From e254733abbbc87808bfef2d3f91f39bc4520e2ac Mon Sep 17 00:00:00 2001 From: Sebastian Florek Date: Tue, 28 Jul 2026 17:21:27 +0200 Subject: [PATCH 3/5] refactor(workbenches): remove pr_followup_service_test.go - Delete file `pr_followup_service_test.go` as it is no longer needed - Modify `pr_followup_service.go` to use `console.ConsoleClient` directly - Simplify client interface in `NewPRFollowupService` function --- .../workbenches/pr_followup_service.go | 10 +- .../workbenches/pr_followup_service_test.go | 135 ------------------ 2 files changed, 4 insertions(+), 141 deletions(-) delete mode 100644 cmd/command/workbenches/pr_followup_service_test.go diff --git a/cmd/command/workbenches/pr_followup_service.go b/cmd/command/workbenches/pr_followup_service.go index 8ee94acf1..8c4cb6e60 100644 --- a/cmd/command/workbenches/pr_followup_service.go +++ b/cmd/command/workbenches/pr_followup_service.go @@ -3,20 +3,18 @@ package workbenches import ( "fmt" "strings" + + "github.com/pluralsh/plural-cli/pkg/console" ) const pullRequestNotFoundError = "pull request not found" -type PRFollowupClient interface { - CreateWorkbenchPRFollowup(url, prompt string) (string, error) -} - type PullRequestURLResolver interface { Resolve(options PullRequestOptions) (string, error) } type PRFollowupService struct { - client PRFollowupClient + client console.ConsoleClient resolver PullRequestURLResolver } @@ -32,7 +30,7 @@ type PRFollowupResult struct { Skipped bool } -func NewPRFollowupService(client PRFollowupClient, resolver PullRequestURLResolver) *PRFollowupService { +func NewPRFollowupService(client console.ConsoleClient, resolver PullRequestURLResolver) *PRFollowupService { return &PRFollowupService{client: client, resolver: resolver} } diff --git a/cmd/command/workbenches/pr_followup_service_test.go b/cmd/command/workbenches/pr_followup_service_test.go deleted file mode 100644 index 1afbfcb44..000000000 --- a/cmd/command/workbenches/pr_followup_service_test.go +++ /dev/null @@ -1,135 +0,0 @@ -package workbenches - -import ( - "errors" - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -type fakePRFollowupClient struct { - activityID string - err error - url string - prompt string - calls int -} - -func (f *fakePRFollowupClient) CreateWorkbenchPRFollowup(url, prompt string) (string, error) { - f.url = url - f.prompt = prompt - f.calls++ - return f.activityID, f.err -} - -type fakePullRequestResolver struct { - url string - err error - options PullRequestOptions - calls int -} - -func (f *fakePullRequestResolver) Resolve(options PullRequestOptions) (string, error) { - f.options = options - f.calls++ - return f.url, f.err -} - -func TestPRFollowupServiceUsesResolvedURLAndClientResponse(t *testing.T) { - client := &fakePRFollowupClient{activityID: "activity-1"} - resolver := &fakePullRequestResolver{url: "https://github.com/pluralsh/plural-cli/pull/5078"} - service := NewPRFollowupService(client, resolver) - options := PRFollowupOptions{ - Prompt: " verify the fix ", - PullRequest: PullRequestOptions{Commit: "HEAD~1"}, - } - - result, err := service.Create(options) - - require.NoError(t, err) - assert.Equal(t, PRFollowupResult{ - ActivityID: "activity-1", - PullRequestURL: resolver.url, - }, result) - assert.Equal(t, options.PullRequest, resolver.options) - assert.Equal(t, resolver.url, client.url) - assert.Equal(t, options.Prompt, client.prompt) - assert.Equal(t, 1, client.calls) -} - -func TestPRFollowupServicePropagatesClientError(t *testing.T) { - client := &fakePRFollowupClient{err: errors.New("job is currently active")} - resolver := &fakePullRequestResolver{url: "https://github.com/pluralsh/plural-cli/pull/5078"} - service := NewPRFollowupService(client, resolver) - - _, err := service.Create(PRFollowupOptions{Prompt: "verify the fix"}) - - require.EqualError(t, err, "job is currently active") -} - -func TestPRFollowupServiceSkipsMissingPullRequest(t *testing.T) { - client := &fakePRFollowupClient{err: errors.New("GraphQL error: pull request not found: WorkbenchPrFollowup")} - resolver := &fakePullRequestResolver{url: "https://github.com/pluralsh/plural-cli/pull/5078"} - service := NewPRFollowupService(client, resolver) - - result, err := service.Create(PRFollowupOptions{ - Prompt: "verify the fix", - SkipMissing: true, - }) - - require.NoError(t, err) - assert.Equal(t, PRFollowupResult{ - PullRequestURL: resolver.url, - Skipped: true, - }, result) - assert.Equal(t, 1, client.calls) -} - -func TestPRFollowupServiceDoesNotSkipOtherErrors(t *testing.T) { - client := &fakePRFollowupClient{err: errors.New("GraphQL error: unauthorized")} - resolver := &fakePullRequestResolver{url: "https://github.com/pluralsh/plural-cli/pull/5078"} - service := NewPRFollowupService(client, resolver) - - _, err := service.Create(PRFollowupOptions{ - Prompt: "verify the fix", - SkipMissing: true, - }) - - require.EqualError(t, err, "GraphQL error: unauthorized") -} - -func TestPRFollowupServiceDoesNotSkipUnrelatedErrorContainingMissingText(t *testing.T) { - client := &fakePRFollowupClient{err: errors.New("failed while checking whether pull request not found should be ignored")} - resolver := &fakePullRequestResolver{url: "https://github.com/pluralsh/plural-cli/pull/5078"} - service := NewPRFollowupService(client, resolver) - - _, err := service.Create(PRFollowupOptions{ - Prompt: "verify the fix", - SkipMissing: true, - }) - - require.EqualError(t, err, "failed while checking whether pull request not found should be ignored") -} - -func TestPRFollowupServiceRejectsEmptyClientResponse(t *testing.T) { - client := &fakePRFollowupClient{} - resolver := &fakePullRequestResolver{url: "https://github.com/pluralsh/plural-cli/pull/5078"} - service := NewPRFollowupService(client, resolver) - - _, err := service.Create(PRFollowupOptions{Prompt: "verify the fix"}) - - require.EqualError(t, err, "console returned an empty workbench PR follow-up response") -} - -func TestPRFollowupServiceValidatesPromptBeforeResolvingURL(t *testing.T) { - client := &fakePRFollowupClient{} - resolver := &fakePullRequestResolver{err: errors.New("should not be called")} - service := NewPRFollowupService(client, resolver) - - _, err := service.Create(PRFollowupOptions{Prompt: " \t "}) - - require.EqualError(t, err, "prompt cannot be empty") - assert.Zero(t, resolver.calls) - assert.Zero(t, client.calls) -} From 8f3097a89b21e44e70bf7231e02d8ddf42fe9f32 Mon Sep 17 00:00:00 2001 From: michaeljguarino Date: Tue, 28 Jul 2026 16:09:48 -0400 Subject: [PATCH 4/5] use the new enqueue pr followup mutation --- .../workbenches/pr_followup_service.go | 10 ++++--- cmd/command/workbenches/workbenches.go | 20 ++++++++++++- cmd/command/workbenches/workbenches_test.go | 11 ++++--- go.mod | 2 +- go.sum | 4 +-- pkg/console/console.go | 2 ++ pkg/console/workbenches.go | 13 ++++++++ pkg/test/mocks/ConsoleClient.go | 30 +++++++++++++++++++ 8 files changed, 80 insertions(+), 12 deletions(-) diff --git a/cmd/command/workbenches/pr_followup_service.go b/cmd/command/workbenches/pr_followup_service.go index 8c4cb6e60..c4bcb213a 100644 --- a/cmd/command/workbenches/pr_followup_service.go +++ b/cmd/command/workbenches/pr_followup_service.go @@ -3,6 +3,7 @@ package workbenches import ( "fmt" "strings" + "time" "github.com/pluralsh/plural-cli/pkg/console" ) @@ -20,12 +21,13 @@ type PRFollowupService struct { type PRFollowupOptions struct { Prompt string + Defer time.Duration PullRequest PullRequestOptions SkipMissing bool } type PRFollowupResult struct { - ActivityID string + PromptID string PullRequestURL string Skipped bool } @@ -50,7 +52,7 @@ func (s *PRFollowupService) Create(options PRFollowupOptions) (PRFollowupResult, return PRFollowupResult{}, err } - activityID, err := s.client.CreateWorkbenchPRFollowup(pullRequestURL, options.Prompt) + promptID, err := s.client.EnqueueWorkbenchPRFollowup(pullRequestURL, options.Prompt, options.Defer) if err != nil { if options.SkipMissing && isPullRequestNotFound(err) { return PRFollowupResult{PullRequestURL: pullRequestURL, Skipped: true}, nil @@ -58,11 +60,11 @@ func (s *PRFollowupService) Create(options PRFollowupOptions) (PRFollowupResult, return PRFollowupResult{}, err } - if activityID == "" { + if promptID == "" { return PRFollowupResult{}, fmt.Errorf("console returned an empty workbench PR follow-up response") } - return PRFollowupResult{ActivityID: activityID, PullRequestURL: pullRequestURL}, nil + return PRFollowupResult{PromptID: promptID, PullRequestURL: pullRequestURL}, nil } func isPullRequestNotFound(err error) bool { diff --git a/cmd/command/workbenches/workbenches.go b/cmd/command/workbenches/workbenches.go index cbbbbabb6..2738225a2 100644 --- a/cmd/command/workbenches/workbenches.go +++ b/cmd/command/workbenches/workbenches.go @@ -1,6 +1,9 @@ package workbenches import ( + "fmt" + "time" + "github.com/urfave/cli" pluralclient "github.com/pluralsh/plural-cli/pkg/client" @@ -74,6 +77,11 @@ func (w *Workbenches) prFollowupCommand() cli.Command { Usage: "source control provider (auto, github, gitlab, or bitbucket)", Value: string(ProviderAuto), }, + cli.StringFlag{ + Name: "defer", + Usage: "Defer the follow-up for a duration, eg 1s, 1m, 2h, etc", + Value: "0s", + }, cli.BoolFlag{ Name: "skip-missing", Usage: "exit successfully when the pull request is not associated with a workbench job", @@ -87,9 +95,19 @@ func (w *Workbenches) handlePRFollowup(ctx *cli.Context) error { return err } + deferDuration, err := time.ParseDuration(ctx.String("defer")) + if err != nil { + return fmt.Errorf("invalid defer duration: %w", err) + } + + if deferDuration < 0 { + return fmt.Errorf("defer duration must be non-negative") + } + service := NewPRFollowupService(w.ConsoleClient, NewPullRequestResolver(nil)) result, err := service.Create(PRFollowupOptions{ Prompt: ctx.String("prompt"), + Defer: deferDuration, SkipMissing: ctx.Bool("skip-missing"), PullRequest: PullRequestOptions{ URL: ctx.String("url"), @@ -106,6 +124,6 @@ func (w *Workbenches) handlePRFollowup(ctx *cli.Context) error { return nil } - utils.Success("Created workbench PR follow-up %s for %s\n", result.ActivityID, result.PullRequestURL) + utils.Success("Created workbench PR follow-up %s for %s\n", result.PromptID, result.PullRequestURL) return nil } diff --git a/cmd/command/workbenches/workbenches_test.go b/cmd/command/workbenches/workbenches_test.go index 2d15f73b5..04a942b1f 100644 --- a/cmd/command/workbenches/workbenches_test.go +++ b/cmd/command/workbenches/workbenches_test.go @@ -4,6 +4,7 @@ import ( "errors" "flag" "testing" + "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" @@ -28,6 +29,7 @@ func TestCommandShape(t *testing.T) { assert.Equal(t, map[string]bool{ "base-url": true, "commit": true, + "defer": true, "prompt": true, "provider": true, "skip-missing": true, @@ -39,9 +41,9 @@ func TestHandlePRFollowupUsesConfiguredConsoleClient(t *testing.T) { url := "https://github.com/pluralsh/plural-cli/pull/5078" prompt := " verify the fix " consoleMock := mocks.NewConsoleClient(t) - consoleMock.On("CreateWorkbenchPRFollowup", url, prompt).Return("activity-1", nil).Once() + consoleMock.On("EnqueueWorkbenchPRFollowup", url, prompt, 2*time.Minute).Return("prompt-1", nil).Once() workbenches := NewWorkbenches(pluralclient.Plural{ConsoleClient: consoleMock}) - ctx := prFollowupContext(t, "--url", url, "--prompt", prompt) + ctx := prFollowupContext(t, "--url", url, "--prompt", prompt, "--defer", "2m") err := workbenches.handlePRFollowup(ctx) @@ -52,7 +54,7 @@ func TestHandlePRFollowupUsesConfiguredConsoleClient(t *testing.T) { func TestHandlePRFollowupPropagatesConsoleError(t *testing.T) { url := "https://github.com/pluralsh/plural-cli/pull/5078" consoleMock := mocks.NewConsoleClient(t) - consoleMock.On("CreateWorkbenchPRFollowup", url, mock.Anything).Return("", errors.New("pull request not found")).Once() + consoleMock.On("EnqueueWorkbenchPRFollowup", url, mock.Anything, time.Duration(0)).Return("", errors.New("pull request not found")).Once() workbenches := NewWorkbenches(pluralclient.Plural{ConsoleClient: consoleMock}) ctx := prFollowupContext(t, "--url", url, "--prompt", "verify") @@ -64,7 +66,7 @@ func TestHandlePRFollowupPropagatesConsoleError(t *testing.T) { func TestHandlePRFollowupSkipsMissingPullRequest(t *testing.T) { url := "https://github.com/pluralsh/plural-cli/pull/5078" consoleMock := mocks.NewConsoleClient(t) - consoleMock.On("CreateWorkbenchPRFollowup", url, mock.Anything).Return("", errors.New("GraphQL error: pull request not found: WorkbenchPrFollowup")).Once() + consoleMock.On("EnqueueWorkbenchPRFollowup", url, mock.Anything, time.Duration(0)).Return("", errors.New("GraphQL error: pull request not found: EnqueueWorkbenchPrFollowup")).Once() workbenches := NewWorkbenches(pluralclient.Plural{ConsoleClient: consoleMock}) ctx := prFollowupContext(t, "--url", url, "--prompt", "verify", "--skip-missing") @@ -92,6 +94,7 @@ func prFollowupContext(t *testing.T, args ...string) *cli.Context { flags.String("base-url", "", "") flags.String("prompt", "", "") flags.String("provider", string(ProviderAuto), "") + flags.String("defer", "0s", "") flags.Bool("skip-missing", false, "") require.NoError(t, flags.Parse(args)) diff --git a/go.mod b/go.mod index 5f81e5dcf..4798faa5f 100644 --- a/go.mod +++ b/go.mod @@ -41,7 +41,7 @@ require ( github.com/mitchellh/mapstructure v1.5.1-0.20231216201459-8508981c8b6c github.com/olekukonko/tablewriter v1.1.4 github.com/pkg/browser v0.0.0-20240102092130-5ac0b6a4141c - github.com/pluralsh/console/go/client v1.78.0 + github.com/pluralsh/console/go/client v1.79.0 github.com/pluralsh/console/go/polly v1.0.0 github.com/pluralsh/gqlclient v1.12.2 github.com/pluralsh/plural-operator v0.6.0 diff --git a/go.sum b/go.sum index b50a9c329..5c14db759 100644 --- a/go.sum +++ b/go.sum @@ -608,8 +608,8 @@ github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4= github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/planetscale/vtprotobuf v0.6.1-0.20240319094008-0393e58bdf10 h1:GFCKgmp0tecUJ0sJuv4pzYCqS9+RGSn52M3FUwPs+uo= github.com/planetscale/vtprotobuf v0.6.1-0.20240319094008-0393e58bdf10/go.mod h1:t/avpk3KcrXxUnYOhZhMXJlSEyie6gQbtLq5NM3loB8= -github.com/pluralsh/console/go/client v1.78.0 h1:gFvXniJOzG0XZpmPAiMCULvzRs8+yN/HpWwoYhymWIg= -github.com/pluralsh/console/go/client v1.78.0/go.mod h1:COAf7KBKIkB9TJfoP25OZU2RWanwHi8Rvzr3/sl1eH0= +github.com/pluralsh/console/go/client v1.79.0 h1:7WeNM5jLymv5V+CGj6Vf0kI+nyhATOIyTzgxZXluVkY= +github.com/pluralsh/console/go/client v1.79.0/go.mod h1:COAf7KBKIkB9TJfoP25OZU2RWanwHi8Rvzr3/sl1eH0= github.com/pluralsh/console/go/controller v0.0.0-20260706120905-5f6ff115eb71 h1:d00SB5O8he1ls7YmUpI10J9FiqXKU+k8EgI0gKqRdSo= github.com/pluralsh/console/go/controller v0.0.0-20260706120905-5f6ff115eb71/go.mod h1:7eM3lngWsj4OUrQsuN2ZTT5UgV7c5yi+PZF8Cer3K+M= github.com/pluralsh/console/go/polly v1.0.0 h1:NRg8CMITGllEJ08oYidNuuG/gJGDdDuga8TM+gdIcFA= diff --git a/pkg/console/console.go b/pkg/console/console.go index 9fa3a5b84..3cae2cd83 100644 --- a/pkg/console/console.go +++ b/pkg/console/console.go @@ -5,6 +5,7 @@ import ( "fmt" "net/http" neturl "net/url" + "time" consoleclient "github.com/pluralsh/console/go/client" ) @@ -61,6 +62,7 @@ type ConsoleClient interface { ListStackRuns(stackID string) (*consoleclient.ListStackRuns, error) CreatePullRequest(id string, branch, context *string) (*consoleclient.PullRequestFragment, error) CreateWorkbenchPRFollowup(url, prompt string) (string, error) + EnqueueWorkbenchPRFollowup(url, prompt string, dur time.Duration) (string, error) GetPrAutomationByName(name string) (*consoleclient.PrAutomationFragment, error) CreateBootstrapToken(attributes consoleclient.BootstrapTokenAttributes) (string, error) CreateClusterRegistration(attributes consoleclient.ClusterRegistrationCreateAttributes) (*consoleclient.ClusterRegistrationFragment, error) diff --git a/pkg/console/workbenches.go b/pkg/console/workbenches.go index 375a02b9a..049783268 100644 --- a/pkg/console/workbenches.go +++ b/pkg/console/workbenches.go @@ -2,6 +2,7 @@ package console import ( "fmt" + "time" consoleclient "github.com/pluralsh/console/go/client" @@ -20,3 +21,15 @@ func (c *consoleClient) CreateWorkbenchPRFollowup(url, prompt string) (string, e return activity.GetID(), nil } + +func (c *consoleClient) EnqueueWorkbenchPRFollowup(url, prompt string, dur time.Duration) (string, error) { + dequeableAt := time.Now().Add(dur) + result, err := c.client.EnqueueWorkbenchPrFollowup(c.ctx, url, consoleclient.QueuedPromptAttributes{ + Prompt: prompt, + DequeableAt: dequeableAt.Format(time.RFC3339), + }) + if err != nil { + return "", api.GetErrorResponse(err, "EnqueueWorkbenchPrFollowup") + } + return result.GetEnqueueWorkbenchPrFollowup().GetID(), nil +} diff --git a/pkg/test/mocks/ConsoleClient.go b/pkg/test/mocks/ConsoleClient.go index bd440a25a..cc5fb3b46 100644 --- a/pkg/test/mocks/ConsoleClient.go +++ b/pkg/test/mocks/ConsoleClient.go @@ -6,6 +6,8 @@ import ( client "github.com/pluralsh/console/go/client" mock "github.com/stretchr/testify/mock" + + time "time" ) // ConsoleClient is an autogenerated mock type for the ConsoleClient type @@ -493,6 +495,34 @@ func (_m *ConsoleClient) DetachCluster(id string) error { return r0 } +// EnqueueWorkbenchPRFollowup provides a mock function with given fields: url, prompt, dur +func (_m *ConsoleClient) EnqueueWorkbenchPRFollowup(url string, prompt string, dur time.Duration) (string, error) { + ret := _m.Called(url, prompt, dur) + + if len(ret) == 0 { + panic("no return value specified for EnqueueWorkbenchPRFollowup") + } + + var r0 string + var r1 error + if rf, ok := ret.Get(0).(func(string, string, time.Duration) (string, error)); ok { + return rf(url, prompt, dur) + } + if rf, ok := ret.Get(0).(func(string, string, time.Duration) string); ok { + r0 = rf(url, prompt, dur) + } else { + r0 = ret.Get(0).(string) + } + + if rf, ok := ret.Get(1).(func(string, string, time.Duration) error); ok { + r1 = rf(url, prompt, dur) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + // ExtUrl provides a mock function with no fields func (_m *ConsoleClient) ExtUrl() string { ret := _m.Called() From b8d239e73e894e4807a92126ab2feaa7c71b0ab1 Mon Sep 17 00:00:00 2001 From: michaeljguarino Date: Tue, 28 Jul 2026 17:14:03 -0400 Subject: [PATCH 5/5] more sophisticated followup outputs --- .../workbenches/pr_followup_service.go | 18 ++--- cmd/command/workbenches/workbenches.go | 30 +++++++- cmd/command/workbenches/workbenches_test.go | 36 ++++++++-- go.mod | 2 +- go.sum | 4 +- pkg/common/output_format.go | 8 +++ pkg/common/string_enum.go | 71 +++++++++++++++++++ pkg/common/string_enum_test.go | 32 +++++++++ pkg/console/console.go | 2 +- pkg/console/workbenches.go | 11 ++- pkg/test/mocks/ConsoleClient.go | 12 ++-- 11 files changed, 199 insertions(+), 27 deletions(-) create mode 100644 pkg/common/output_format.go create mode 100644 pkg/common/string_enum.go create mode 100644 pkg/common/string_enum_test.go diff --git a/cmd/command/workbenches/pr_followup_service.go b/cmd/command/workbenches/pr_followup_service.go index c4bcb213a..1d82b5bca 100644 --- a/cmd/command/workbenches/pr_followup_service.go +++ b/cmd/command/workbenches/pr_followup_service.go @@ -27,9 +27,10 @@ type PRFollowupOptions struct { } type PRFollowupResult struct { - PromptID string - PullRequestURL string - Skipped bool + PromptID string + PullRequestURL string + WorkbenchJobURL string + Skipped bool } func NewPRFollowupService(client console.ConsoleClient, resolver PullRequestURLResolver) *PRFollowupService { @@ -52,7 +53,7 @@ func (s *PRFollowupService) Create(options PRFollowupOptions) (PRFollowupResult, return PRFollowupResult{}, err } - promptID, err := s.client.EnqueueWorkbenchPRFollowup(pullRequestURL, options.Prompt, options.Defer) + result, err := s.client.EnqueueWorkbenchPRFollowup(pullRequestURL, options.Prompt, options.Defer) if err != nil { if options.SkipMissing && isPullRequestNotFound(err) { return PRFollowupResult{PullRequestURL: pullRequestURL, Skipped: true}, nil @@ -60,11 +61,12 @@ func (s *PRFollowupService) Create(options PRFollowupOptions) (PRFollowupResult, return PRFollowupResult{}, err } - if promptID == "" { - return PRFollowupResult{}, fmt.Errorf("console returned an empty workbench PR follow-up response") - } - return PRFollowupResult{PromptID: promptID, PullRequestURL: pullRequestURL}, nil + return PRFollowupResult{ + PromptID: result.GetID(), + PullRequestURL: pullRequestURL, + WorkbenchJobURL: result.GetWorkbenchJob().GetURL(), + }, nil } func isPullRequestNotFound(err error) bool { diff --git a/cmd/command/workbenches/workbenches.go b/cmd/command/workbenches/workbenches.go index 2738225a2..ab7ff77e6 100644 --- a/cmd/command/workbenches/workbenches.go +++ b/cmd/command/workbenches/workbenches.go @@ -1,7 +1,9 @@ package workbenches import ( + "encoding/json" "fmt" + "os" "time" "github.com/urfave/cli" @@ -82,6 +84,7 @@ func (w *Workbenches) prFollowupCommand() cli.Command { Usage: "Defer the follow-up for a duration, eg 1s, 1m, 2h, etc", Value: "0s", }, + common.StringEnumFlag("output, o", "output format", common.OutputFormatRaw, common.OutputFormats...), cli.BoolFlag{ Name: "skip-missing", Usage: "exit successfully when the pull request is not associated with a workbench job", @@ -104,6 +107,11 @@ func (w *Workbenches) handlePRFollowup(ctx *cli.Context) error { return fmt.Errorf("defer duration must be non-negative") } + output := ctx.String("output") + if err := common.ValidateStringEnum("output", output, common.OutputFormats...); err != nil { + return err + } + service := NewPRFollowupService(w.ConsoleClient, NewPullRequestResolver(nil)) result, err := service.Create(PRFollowupOptions{ Prompt: ctx.String("prompt"), @@ -119,11 +127,27 @@ func (w *Workbenches) handlePRFollowup(ctx *cli.Context) error { if err != nil { return err } + + return writePRFollowupResult(output, result) +} + +func writePRFollowupResult(output string, result PRFollowupResult) error { + switch output { + case common.OutputFormatRaw: + writeRawPRFollowupResult(result) + case common.OutputFormatJSON: + return json.NewEncoder(os.Stdout).Encode(result) + } + + return nil +} + +func writeRawPRFollowupResult(result PRFollowupResult) { if result.Skipped { utils.Success("No workbench job found for %s; skipping\n", result.PullRequestURL) - return nil + return } - utils.Success("Created workbench PR follow-up %s for %s\n", result.PromptID, result.PullRequestURL) - return nil + fmt.Printf("Created workbench PR follow-up %s for %s\n", result.PromptID, result.PullRequestURL) + utils.Success("Workbench Job URL: %s\n", result.WorkbenchJobURL) } diff --git a/cmd/command/workbenches/workbenches_test.go b/cmd/command/workbenches/workbenches_test.go index 04a942b1f..a22cb3cd4 100644 --- a/cmd/command/workbenches/workbenches_test.go +++ b/cmd/command/workbenches/workbenches_test.go @@ -3,9 +3,11 @@ package workbenches import ( "errors" "flag" + "strings" "testing" "time" + consoleclient "github.com/pluralsh/console/go/client" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" @@ -30,6 +32,8 @@ func TestCommandShape(t *testing.T) { "base-url": true, "commit": true, "defer": true, + "o": true, + "output": true, "prompt": true, "provider": true, "skip-missing": true, @@ -41,7 +45,7 @@ func TestHandlePRFollowupUsesConfiguredConsoleClient(t *testing.T) { url := "https://github.com/pluralsh/plural-cli/pull/5078" prompt := " verify the fix " consoleMock := mocks.NewConsoleClient(t) - consoleMock.On("EnqueueWorkbenchPRFollowup", url, prompt, 2*time.Minute).Return("prompt-1", nil).Once() + consoleMock.On("EnqueueWorkbenchPRFollowup", url, prompt, 2*time.Minute).Return(enqueuedPRFollowup("prompt-1", "https://console.example.com/workbenches/jobs/job-1"), nil).Once() workbenches := NewWorkbenches(pluralclient.Plural{ConsoleClient: consoleMock}) ctx := prFollowupContext(t, "--url", url, "--prompt", prompt, "--defer", "2m") @@ -54,7 +58,7 @@ func TestHandlePRFollowupUsesConfiguredConsoleClient(t *testing.T) { func TestHandlePRFollowupPropagatesConsoleError(t *testing.T) { url := "https://github.com/pluralsh/plural-cli/pull/5078" consoleMock := mocks.NewConsoleClient(t) - consoleMock.On("EnqueueWorkbenchPRFollowup", url, mock.Anything, time.Duration(0)).Return("", errors.New("pull request not found")).Once() + consoleMock.On("EnqueueWorkbenchPRFollowup", url, mock.Anything, time.Duration(0)).Return(nil, errors.New("pull request not found")).Once() workbenches := NewWorkbenches(pluralclient.Plural{ConsoleClient: consoleMock}) ctx := prFollowupContext(t, "--url", url, "--prompt", "verify") @@ -66,7 +70,7 @@ func TestHandlePRFollowupPropagatesConsoleError(t *testing.T) { func TestHandlePRFollowupSkipsMissingPullRequest(t *testing.T) { url := "https://github.com/pluralsh/plural-cli/pull/5078" consoleMock := mocks.NewConsoleClient(t) - consoleMock.On("EnqueueWorkbenchPRFollowup", url, mock.Anything, time.Duration(0)).Return("", errors.New("GraphQL error: pull request not found: EnqueueWorkbenchPrFollowup")).Once() + consoleMock.On("EnqueueWorkbenchPRFollowup", url, mock.Anything, time.Duration(0)).Return(nil, errors.New("GraphQL error: pull request not found: EnqueueWorkbenchPrFollowup")).Once() workbenches := NewWorkbenches(pluralclient.Plural{ConsoleClient: consoleMock}) ctx := prFollowupContext(t, "--url", url, "--prompt", "verify", "--skip-missing") @@ -76,10 +80,23 @@ func TestHandlePRFollowupSkipsMissingPullRequest(t *testing.T) { consoleMock.AssertExpectations(t) } +func TestHandlePRFollowupRejectsUnsupportedOutput(t *testing.T) { + consoleMock := mocks.NewConsoleClient(t) + workbenches := NewWorkbenches(pluralclient.Plural{ConsoleClient: consoleMock}) + ctx := prFollowupContext(t, "--url", "https://github.com/pluralsh/plural-cli/pull/5078", "--prompt", "verify", "--output", "yaml") + + err := workbenches.handlePRFollowup(ctx) + + require.EqualError(t, err, `unsupported output "yaml" (must be one of: raw, json)`) + consoleMock.AssertNotCalled(t, "EnqueueWorkbenchPRFollowup", mock.Anything, mock.Anything, mock.Anything) +} + func flagNames(flags []cli.Flag) map[string]bool { names := make(map[string]bool, len(flags)) for _, commandFlag := range flags { - names[commandFlag.GetName()] = true + for _, name := range strings.Split(commandFlag.GetName(), ",") { + names[strings.TrimSpace(name)] = true + } } return names @@ -95,8 +112,19 @@ func prFollowupContext(t *testing.T, args ...string) *cli.Context { flags.String("prompt", "", "") flags.String("provider", string(ProviderAuto), "") flags.String("defer", "0s", "") + flags.String("output", "raw", "") + flags.String("o", "raw", "") flags.Bool("skip-missing", false, "") require.NoError(t, flags.Parse(args)) return cli.NewContext(nil, flags, nil) } + +func enqueuedPRFollowup(id, workbenchJobURL string) *consoleclient.EnqueueWorkbenchPrFollowup_EnqueueWorkbenchPrFollowup { + return &consoleclient.EnqueueWorkbenchPrFollowup_EnqueueWorkbenchPrFollowup{ + ID: id, + WorkbenchJob: &consoleclient.EnqueueWorkbenchPrFollowup_EnqueueWorkbenchPrFollowup_WorkbenchJob{ + URL: workbenchJobURL, + }, + } +} diff --git a/go.mod b/go.mod index 4798faa5f..a578baee8 100644 --- a/go.mod +++ b/go.mod @@ -41,7 +41,7 @@ require ( github.com/mitchellh/mapstructure v1.5.1-0.20231216201459-8508981c8b6c github.com/olekukonko/tablewriter v1.1.4 github.com/pkg/browser v0.0.0-20240102092130-5ac0b6a4141c - github.com/pluralsh/console/go/client v1.79.0 + github.com/pluralsh/console/go/client v1.79.1 github.com/pluralsh/console/go/polly v1.0.0 github.com/pluralsh/gqlclient v1.12.2 github.com/pluralsh/plural-operator v0.6.0 diff --git a/go.sum b/go.sum index 5c14db759..2c393e7c8 100644 --- a/go.sum +++ b/go.sum @@ -608,8 +608,8 @@ github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4= github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/planetscale/vtprotobuf v0.6.1-0.20240319094008-0393e58bdf10 h1:GFCKgmp0tecUJ0sJuv4pzYCqS9+RGSn52M3FUwPs+uo= github.com/planetscale/vtprotobuf v0.6.1-0.20240319094008-0393e58bdf10/go.mod h1:t/avpk3KcrXxUnYOhZhMXJlSEyie6gQbtLq5NM3loB8= -github.com/pluralsh/console/go/client v1.79.0 h1:7WeNM5jLymv5V+CGj6Vf0kI+nyhATOIyTzgxZXluVkY= -github.com/pluralsh/console/go/client v1.79.0/go.mod h1:COAf7KBKIkB9TJfoP25OZU2RWanwHi8Rvzr3/sl1eH0= +github.com/pluralsh/console/go/client v1.79.1 h1:W9EBknG+1SlKMml4bgMQXZ4jMcLT8ElFKwyTZDzRDdA= +github.com/pluralsh/console/go/client v1.79.1/go.mod h1:COAf7KBKIkB9TJfoP25OZU2RWanwHi8Rvzr3/sl1eH0= github.com/pluralsh/console/go/controller v0.0.0-20260706120905-5f6ff115eb71 h1:d00SB5O8he1ls7YmUpI10J9FiqXKU+k8EgI0gKqRdSo= github.com/pluralsh/console/go/controller v0.0.0-20260706120905-5f6ff115eb71/go.mod h1:7eM3lngWsj4OUrQsuN2ZTT5UgV7c5yi+PZF8Cer3K+M= github.com/pluralsh/console/go/polly v1.0.0 h1:NRg8CMITGllEJ08oYidNuuG/gJGDdDuga8TM+gdIcFA= diff --git a/pkg/common/output_format.go b/pkg/common/output_format.go new file mode 100644 index 000000000..194ef3c43 --- /dev/null +++ b/pkg/common/output_format.go @@ -0,0 +1,8 @@ +package common + +const ( + OutputFormatRaw = "raw" + OutputFormatJSON = "json" +) + +var OutputFormats = []string{OutputFormatRaw, OutputFormatJSON} diff --git a/pkg/common/string_enum.go b/pkg/common/string_enum.go new file mode 100644 index 000000000..b175f65a1 --- /dev/null +++ b/pkg/common/string_enum.go @@ -0,0 +1,71 @@ +package common + +import ( + "fmt" + "strings" + + "github.com/urfave/cli" +) + +type StringEnum struct { + name string + value string + allowed []string +} + +func NewStringEnum(name, defaultValue string, allowed ...string) *StringEnum { + enum := &StringEnum{name: name, allowed: allowed} + if err := enum.Set(defaultValue); err != nil { + panic(err) + } + + return enum +} + +func StringEnumFlag(name, usage, defaultValue string, allowed ...string) cli.GenericFlag { + return cli.GenericFlag{ + Name: name, + Usage: fmt.Sprintf("%s (%s)", usage, strings.Join(allowed, " or ")), + Value: NewStringEnum(primaryFlagName(name), defaultValue, allowed...), + } +} + +func ValidateStringEnum(name, value string, allowed ...string) error { + return validateStringEnumValue(name, value, allowed) +} + +func (e *StringEnum) Set(value string) error { + if err := validateStringEnumValue(e.name, value, e.allowed); err != nil { + return err + } + + e.value = value + return nil +} + +func (e *StringEnum) String() string { + if e == nil { + return "" + } + + return e.value +} + +func validateStringEnumValue(name, value string, allowed []string) error { + if len(allowed) == 0 { + return fmt.Errorf("no allowed values configured for %s", name) + } + + for _, option := range allowed { + if value == option { + return nil + } + } + + return fmt.Errorf("unsupported %s %q (must be one of: %s)", name, value, strings.Join(allowed, ", ")) +} + +func primaryFlagName(name string) string { + primary, _, _ := strings.Cut(name, ",") + return strings.TrimSpace(primary) +} diff --git a/pkg/common/string_enum_test.go b/pkg/common/string_enum_test.go new file mode 100644 index 000000000..558b03a5b --- /dev/null +++ b/pkg/common/string_enum_test.go @@ -0,0 +1,32 @@ +package common + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestStringEnumAcceptsAllowedValues(t *testing.T) { + enum := NewStringEnum("output", "raw", "raw", "json") + + require.Equal(t, "raw", enum.String()) + require.NoError(t, enum.Set("json")) + require.Equal(t, "json", enum.String()) +} + +func TestStringEnumRejectsUnsupportedValues(t *testing.T) { + enum := NewStringEnum("output", "raw", "raw", "json") + + err := enum.Set("yaml") + + require.EqualError(t, err, `unsupported output "yaml" (must be one of: raw, json)`) + require.Equal(t, "raw", enum.String()) +} + +func TestStringEnumFlagUsesPrimaryNameInErrors(t *testing.T) { + flag := StringEnumFlag("output, o", "output format", "raw", "raw", "json") + + err := flag.Value.Set("yaml") + + require.EqualError(t, err, `unsupported output "yaml" (must be one of: raw, json)`) +} diff --git a/pkg/console/console.go b/pkg/console/console.go index 3cae2cd83..db7f144c3 100644 --- a/pkg/console/console.go +++ b/pkg/console/console.go @@ -62,7 +62,7 @@ type ConsoleClient interface { ListStackRuns(stackID string) (*consoleclient.ListStackRuns, error) CreatePullRequest(id string, branch, context *string) (*consoleclient.PullRequestFragment, error) CreateWorkbenchPRFollowup(url, prompt string) (string, error) - EnqueueWorkbenchPRFollowup(url, prompt string, dur time.Duration) (string, error) + EnqueueWorkbenchPRFollowup(url, prompt string, dur time.Duration) (*consoleclient.EnqueueWorkbenchPrFollowup_EnqueueWorkbenchPrFollowup, error) GetPrAutomationByName(name string) (*consoleclient.PrAutomationFragment, error) CreateBootstrapToken(attributes consoleclient.BootstrapTokenAttributes) (string, error) CreateClusterRegistration(attributes consoleclient.ClusterRegistrationCreateAttributes) (*consoleclient.ClusterRegistrationFragment, error) diff --git a/pkg/console/workbenches.go b/pkg/console/workbenches.go index 049783268..b1ad0c68e 100644 --- a/pkg/console/workbenches.go +++ b/pkg/console/workbenches.go @@ -22,14 +22,19 @@ func (c *consoleClient) CreateWorkbenchPRFollowup(url, prompt string) (string, e return activity.GetID(), nil } -func (c *consoleClient) EnqueueWorkbenchPRFollowup(url, prompt string, dur time.Duration) (string, error) { +func (c *consoleClient) EnqueueWorkbenchPRFollowup(url, prompt string, dur time.Duration) (*consoleclient.EnqueueWorkbenchPrFollowup_EnqueueWorkbenchPrFollowup, error) { dequeableAt := time.Now().Add(dur) result, err := c.client.EnqueueWorkbenchPrFollowup(c.ctx, url, consoleclient.QueuedPromptAttributes{ Prompt: prompt, DequeableAt: dequeableAt.Format(time.RFC3339), }) if err != nil { - return "", api.GetErrorResponse(err, "EnqueueWorkbenchPrFollowup") + return nil, api.GetErrorResponse(err, "EnqueueWorkbenchPrFollowup") } - return result.GetEnqueueWorkbenchPrFollowup().GetID(), nil + fragment := result.GetEnqueueWorkbenchPrFollowup() + if fragment == nil { + return nil, fmt.Errorf("enqueued prompt fragment failed") + } + + return fragment, nil } diff --git a/pkg/test/mocks/ConsoleClient.go b/pkg/test/mocks/ConsoleClient.go index cc5fb3b46..8837fce42 100644 --- a/pkg/test/mocks/ConsoleClient.go +++ b/pkg/test/mocks/ConsoleClient.go @@ -496,22 +496,24 @@ func (_m *ConsoleClient) DetachCluster(id string) error { } // EnqueueWorkbenchPRFollowup provides a mock function with given fields: url, prompt, dur -func (_m *ConsoleClient) EnqueueWorkbenchPRFollowup(url string, prompt string, dur time.Duration) (string, error) { +func (_m *ConsoleClient) EnqueueWorkbenchPRFollowup(url string, prompt string, dur time.Duration) (*client.EnqueueWorkbenchPrFollowup_EnqueueWorkbenchPrFollowup, error) { ret := _m.Called(url, prompt, dur) if len(ret) == 0 { panic("no return value specified for EnqueueWorkbenchPRFollowup") } - var r0 string + var r0 *client.EnqueueWorkbenchPrFollowup_EnqueueWorkbenchPrFollowup var r1 error - if rf, ok := ret.Get(0).(func(string, string, time.Duration) (string, error)); ok { + if rf, ok := ret.Get(0).(func(string, string, time.Duration) (*client.EnqueueWorkbenchPrFollowup_EnqueueWorkbenchPrFollowup, error)); ok { return rf(url, prompt, dur) } - if rf, ok := ret.Get(0).(func(string, string, time.Duration) string); ok { + if rf, ok := ret.Get(0).(func(string, string, time.Duration) *client.EnqueueWorkbenchPrFollowup_EnqueueWorkbenchPrFollowup); ok { r0 = rf(url, prompt, dur) } else { - r0 = ret.Get(0).(string) + if ret.Get(0) != nil { + r0 = ret.Get(0).(*client.EnqueueWorkbenchPrFollowup_EnqueueWorkbenchPrFollowup) + } } if rf, ok := ret.Get(1).(func(string, string, time.Duration) error); ok {