From d700d9e7ecc60b63c8b84189aeb5f906fb1ef1ca Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 11 Aug 2026 23:11:53 +0000 Subject: [PATCH 1/3] Initial plan From b806b1e930b14adc1134f712673d2f9846b5147b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 11 Aug 2026 23:23:08 +0000 Subject: [PATCH 2/3] Improve CLI error message compliance Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/add_interactive_orchestrator.go | 10 +++++----- pkg/cli/add_workflow_resolution.go | 10 +++++----- pkg/cli/audit_diff_command.go | 10 +++++----- pkg/cli/interactive.go | 10 +++++----- pkg/cli/interactive_test.go | 5 ++++- pkg/cli/org_runner.go | 10 +++++----- 6 files changed, 29 insertions(+), 26 deletions(-) diff --git a/pkg/cli/add_interactive_orchestrator.go b/pkg/cli/add_interactive_orchestrator.go index 26560822aa0..2cc9fc57e0f 100644 --- a/pkg/cli/add_interactive_orchestrator.go +++ b/pkg/cli/add_interactive_orchestrator.go @@ -76,7 +76,7 @@ func RunAddInteractive(ctx context.Context, config *AddInteractiveConfig) error // are treated consistently across test and automation environments, while // IsRunningInCI centralizes the broader CI environment detection logic. if envutil.GetBoolFromEnv("GO_TEST_MODE", false, addInteractiveLog) || IsRunningInCI() { - return errors.New("interactive add cannot be used in automated tests or CI environments") + return errors.New("interactive add is unavailable in automated tests or CI environments. Expected an interactive terminal. Example: unset CI and run the command from a terminal") } // Set context on the config @@ -248,11 +248,11 @@ func (c *AddInteractiveConfig) determineFilesToAdd() (workflowFiles []string, in return nil, nil, fmt.Errorf("resolved workflow at position %d from %q is nil", i+1, workflowSpecsForError) } if rw.Spec == nil { - return nil, nil, fmt.Errorf("resolved workflow at position %d from %q is missing its specification", i+1, workflowSpecsForError) + return nil, nil, fmt.Errorf("resolved workflow at position %d from %q has no specification. Expected a resolved workflow specification. Example: github/gh-aw/example-workflow", i+1, workflowSpecsForError) } workflowName := strings.TrimSpace(rw.Spec.WorkflowName) if workflowName == "" { - return nil, nil, fmt.Errorf("resolved workflow at position %d from %q is missing its workflow name", i+1, workflowSpecsForError) + return nil, nil, fmt.Errorf("resolved workflow at position %d from %q has no workflow name. Expected a named resolved workflow. Example: github/gh-aw/example-workflow", i+1, workflowSpecsForError) } if rw.IsActionWorkflow { // Raw GitHub Actions YAML files are installed as-is; no .lock.yml is produced. @@ -293,11 +293,11 @@ func (c *AddInteractiveConfig) workflowNamesForInteractiveAdd() ([]string, error return nil, fmt.Errorf("resolved manifest workflow at position %d from %q is nil", i+1, workflowSpecsForError) } if resolvedWorkflow.Spec == nil { - return nil, fmt.Errorf("resolved manifest workflow at position %d from %q is missing its specification", i+1, workflowSpecsForError) + return nil, fmt.Errorf("resolved manifest workflow at position %d from %q has no specification. Expected a resolved workflow specification. Example: github/gh-aw/example-workflow", i+1, workflowSpecsForError) } workflowName := strings.TrimSpace(resolvedWorkflow.Spec.WorkflowName) if workflowName == "" { - return nil, fmt.Errorf("resolved manifest workflow at position %d from %q is missing its workflow name", i+1, workflowSpecsForError) + return nil, fmt.Errorf("resolved manifest workflow at position %d from %q has no workflow name. Expected a named resolved workflow. Example: github/gh-aw/example-workflow", i+1, workflowSpecsForError) } workflowNames = append(workflowNames, workflowName) } diff --git a/pkg/cli/add_workflow_resolution.go b/pkg/cli/add_workflow_resolution.go index 900d357f3ba..f340eda912c 100644 --- a/pkg/cli/add_workflow_resolution.go +++ b/pkg/cli/add_workflow_resolution.go @@ -126,11 +126,11 @@ type specResolutionResult struct { func validateResolveWorkflowsInput(workflows []string) error { if len(workflows) == 0 { - return errors.New("at least one workflow name is required") + return errors.New("no workflow names were supplied. Expected at least one workflow name. Example: gh aw add github/gh-aw/example-workflow") } for i, workflow := range workflows { if workflow == "" { - return fmt.Errorf("workflow name cannot be empty (workflow %d)", i+1) + return fmt.Errorf("workflow name at position %d is empty. Expected a workflow name. Example: github/gh-aw/example-workflow", i+1) } } return nil @@ -260,7 +260,7 @@ func validateCurrentRepositorySpecs(parsedSpecs []*WorkflowSpec) error { continue } if spec.RepoSlug == currentRepoSlug { - return fmt.Errorf("cannot add workflows from the current repository (%s). The 'add' command is for installing workflows from other repositories", currentRepoSlug) + return fmt.Errorf("workflow source %q is the current repository. Expected a workflow from another repository. Example: gh aw add github/gh-aw/example-workflow", currentRepoSlug) } } return nil @@ -369,7 +369,7 @@ func resolveStandardWorkflow(spec, resolvedSpec *WorkflowSpec, fetched *FetchedW } if ExtractWorkflowPrivate(content) { - return nil, fmt.Errorf("workflow '%s' is private and cannot be added to other repositories", spec.String()) + return nil, fmt.Errorf("workflow %q is private. Expected a workflow that can be added to another repository. Example: set private: false before running gh aw add", spec.String()) } workflowHasDispatch := checkWorkflowHasDispatchFromContent(content) @@ -408,7 +408,7 @@ func validateManifestWorkflowPrivateSetting(spec, resolvedSpec *WorkflowSpec, co } manifestPath := joinRepositoryPackagePath(spec.PackagePath, repositoryPackageManifestFileName) return fmt.Errorf( - "invalid Agentic Workflow manifest %q: workflow %q sets private: true and cannot be included because private workflows cannot be added", + "agentic workflow manifest %q sets workflow %q to private: true. Expected an installable workflow with private: false. Example:\nprivate: false", manifestPath, resolvedSpec.WorkflowPath, ) diff --git a/pkg/cli/audit_diff_command.go b/pkg/cli/audit_diff_command.go index b36ed5e9024..23dc50c7230 100644 --- a/pkg/cli/audit_diff_command.go +++ b/pkg/cli/audit_diff_command.go @@ -50,7 +50,7 @@ analyzes their data, and produces a diff showing: RunE: func(cmd *cobra.Command, args []string) error { baseRunID, err := strconv.ParseInt(args[0], 10, 64) if err != nil { - return fmt.Errorf("invalid base run ID %q: must be a numeric run ID", args[0]) + return fmt.Errorf("base run ID %q is not numeric. Expected a numeric GitHub Actions run ID. Example: gh aw audit diff 12345 12346", args[0]) } compareRunIDs := make([]int64, 0, len(args)-1) @@ -58,13 +58,13 @@ analyzes their data, and produces a diff showing: for _, arg := range args[1:] { id, err := strconv.ParseInt(arg, 10, 64) if err != nil { - return fmt.Errorf("invalid run ID %q: must be a numeric run ID", arg) + return fmt.Errorf("comparison run ID %q is not numeric. Expected a numeric GitHub Actions run ID. Example: gh aw audit diff 12345 12346", arg) } if id == baseRunID { - return fmt.Errorf("comparison run ID %d is the same as the base run ID: cannot diff a run against itself", id) + return fmt.Errorf("comparison run ID %d matches the base run ID. Expected a different run ID for comparison. Example: gh aw audit diff 12345 12346", id) } if seen[id] { - return fmt.Errorf("duplicate comparison run ID %d: each run ID must appear only once", id) + return fmt.Errorf("comparison run ID %d appears more than once. Expected each comparison run ID once. Example: gh aw audit diff 12345 12346", id) } seen[id] = true compareRunIDs = append(compareRunIDs, id) @@ -81,7 +81,7 @@ analyzes their data, and produces a diff showing: if repoFlag != "" { parts := strings.SplitN(repoFlag, "/", 2) if len(parts) != 2 || parts[0] == "" || parts[1] == "" { - return fmt.Errorf("invalid repository format '%s': expected 'owner/repo'", repoFlag) + return fmt.Errorf("repository %q is not in owner/repo format. Expected an owner and repository name separated by '/'. Example: --repo github/gh-aw", repoFlag) } owner = parts[0] repo = parts[1] diff --git a/pkg/cli/interactive.go b/pkg/cli/interactive.go index ed0c8a38d9d..8ac7bb41131 100644 --- a/pkg/cli/interactive.go +++ b/pkg/cli/interactive.go @@ -69,7 +69,7 @@ func CreateWorkflowInteractively(ctx context.Context, workflowName string, verbo // are treated consistently across test and automation environments, while // IsRunningInCI centralizes the broader CI environment detection logic. if envutil.GetBoolFromEnv("GO_TEST_MODE", false, interactiveLog) || IsRunningInCI() { - return errors.New("interactive workflow creation cannot be used in automated tests or CI environments") + return errors.New("interactive workflow creation is unavailable in automated tests or CI environments. Expected an interactive terminal. Example: unset CI and run the command from a terminal") } if verbose { @@ -400,7 +400,7 @@ func promptNonInteractiveSelect(scanner *bufio.Scanner, title string, options [] // Accept a numeric index if idx, err := strconv.Atoi(input); err == nil { if idx < 1 || idx > len(options) { - return "", fmt.Errorf("selection out of range (must be 1-%d)", len(options)) + return "", fmt.Errorf("selection is out of range. Expected a number from 1 to %d. Example: 1", len(options)) } return options[idx-1].value, nil } @@ -411,7 +411,7 @@ func promptNonInteractiveSelect(scanner *bufio.Scanner, title string, options [] return opt.value, nil } } - return "", fmt.Errorf("invalid selection %q", input) + return "", fmt.Errorf("selection %q is not available. Expected a displayed option name or number. Example: 1", input) } // promptNonInteractiveMultiSelect prints a numbered list and reads comma-separated selections. @@ -453,7 +453,7 @@ func promptNonInteractiveMultiSelect(scanner *bufio.Scanner, title string, optio // Try numeric index if idx, err := strconv.Atoi(tok); err == nil { if idx < 1 || idx > len(options) { - return nil, fmt.Errorf("selection %d out of range (must be 1-%d)", idx, len(options)) + return nil, fmt.Errorf("selection %d is out of range. Expected a number from 1 to %d. Example: 1", idx, len(options)) } val := options[idx-1].value if _, dup := seen[val]; !dup { @@ -472,7 +472,7 @@ func promptNonInteractiveMultiSelect(scanner *bufio.Scanner, title string, optio continue } - return nil, fmt.Errorf("unknown option %q", tok) + return nil, fmt.Errorf("option %q is not available. Expected a displayed option name or number. Example: 1", tok) } return selected, nil } diff --git a/pkg/cli/interactive_test.go b/pkg/cli/interactive_test.go index 5c2a04edc92..d7880089d4b 100644 --- a/pkg/cli/interactive_test.go +++ b/pkg/cli/interactive_test.go @@ -438,10 +438,13 @@ func TestCreateWorkflowInteractively_InAutomatedEnvironment(t *testing.T) { t.Error("Expected error in automated environment, got nil") } - expectedErrMsg := "interactive workflow creation cannot be used in automated tests or CI environments" + expectedErrMsg := "interactive workflow creation is unavailable in automated tests or CI environments" if !strings.Contains(err.Error(), expectedErrMsg) { t.Errorf("Expected error containing %q, got %q", expectedErrMsg, err.Error()) } + if !strings.Contains(err.Error(), "Expected an interactive terminal") || !strings.Contains(err.Error(), "Example: unset CI") { + t.Errorf("Expected actionable error message, got %q", err.Error()) + } } func TestCreateWorkflowInteractively_WithForceFlag(t *testing.T) { diff --git a/pkg/cli/org_runner.go b/pkg/cli/org_runner.go index dd4e4eb51bc..f3f7b359bf3 100644 --- a/pkg/cli/org_runner.go +++ b/pkg/cli/org_runner.go @@ -146,19 +146,19 @@ func runCommandForOrg(ctx context.Context, org string, repoGlobs []string, cbs o return errors.New("createPR and createIssue are mutually exclusive") } if cbs.SearchFn == nil { - return errors.New("orgRunCallbacks.SearchFn is required") + return errors.New("organization search callback is not configured. Expected orgRunCallbacks.SearchFn to search organization repositories. Example: orgRunCallbacks{SearchFn: searchFn}") } if cbs.ReportFn == nil { - return errors.New("orgRunCallbacks.ReportFn is required") + return errors.New("organization report callback is not configured. Expected orgRunCallbacks.ReportFn to display the run summary. Example: orgRunCallbacks{ReportFn: reportFn}") } if createPR && cbs.ApplyFn == nil { - return errors.New("orgRunCallbacks.ApplyFn is required when createPR is true") + return errors.New("pull request callback is not configured. Expected orgRunCallbacks.ApplyFn when createPR is enabled. Example: orgRunCallbacks{ApplyFn: applyFn}") } if createIssue && cbs.IssueFn == nil { - return errors.New("orgRunCallbacks.IssueFn is required when createIssue is true") + return errors.New("issue callback is not configured. Expected orgRunCallbacks.IssueFn when createIssue is enabled. Example: orgRunCallbacks{IssueFn: issueFn}") } if (createPR || createIssue) && !cbs.AutoYes && isRunningInCIFn() { - return errors.New("confirmation is required for --org create operations in CI; re-run with --yes to auto-accept") + return errors.New("organization create operations in CI need confirmation. Expected --yes to auto-accept in non-interactive environments. Example: gh aw run --org octo-org --yes") } // Handle Ctrl-C / SIGTERM so an interrupted run still renders the report From 61891c4e786d385a22866edcfc745896e8fcc0c1 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 12 Aug 2026 00:09:40 +0000 Subject: [PATCH 3/3] Address review feedback on actionable errors Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com> --- pkg/cli/add_current_repo_test.go | 6 ++ pkg/cli/add_interactive_orchestrator.go | 2 +- pkg/cli/add_interactive_orchestrator_test.go | 21 +++++ pkg/cli/add_package_manifest_test.go | 3 + pkg/cli/add_workflow_resolution.go | 6 +- pkg/cli/audit_diff_test.go | 42 ++++++++++ pkg/cli/interactive.go | 8 +- pkg/cli/interactive_test.go | 11 ++- pkg/cli/org_runner.go | 10 +-- pkg/cli/org_runner_test.go | 82 ++++++++++++++++++++ pkg/workflow/call_workflow_validation.go | 6 +- 11 files changed, 180 insertions(+), 17 deletions(-) create mode 100644 pkg/cli/org_runner_test.go diff --git a/pkg/cli/add_current_repo_test.go b/pkg/cli/add_current_repo_test.go index 433b2fc6246..011f2d6fc2e 100644 --- a/pkg/cli/add_current_repo_test.go +++ b/pkg/cli/add_current_repo_test.go @@ -80,6 +80,9 @@ func TestAddWorkflowsFromCurrentRepository(t *testing.T) { if !strings.Contains(err.Error(), tt.errorContains) { t.Errorf("Expected error to contain %q, got: %v", tt.errorContains, err) } + if !strings.Contains(err.Error(), "Expected a workflow from another repository") || !strings.Contains(err.Error(), "Example: gh aw add octo-org/agentic-workflows/example-workflow") { + t.Errorf("Expected actionable current repository error, got: %v", err) + } } else { // For "allow" case, we expect a different error (workflow not found, not current repo error) if err != nil && strings.Contains(err.Error(), "cannot add workflows from the current repository") { @@ -164,6 +167,9 @@ func TestAddWorkflowsFromCurrentRepositoryMultiple(t *testing.T) { if !strings.Contains(err.Error(), tt.errorContains) { t.Errorf("Expected error to contain %q, got: %v", tt.errorContains, err) } + if !strings.Contains(err.Error(), "Expected a workflow from another repository") || !strings.Contains(err.Error(), "Example: gh aw add octo-org/agentic-workflows/example-workflow") { + t.Errorf("Expected actionable current repository error, got: %v", err) + } } else { // For "allow" case, we expect a different error (workflow not found, not current repo error) if err != nil && strings.Contains(err.Error(), "cannot add workflows from the current repository") { diff --git a/pkg/cli/add_interactive_orchestrator.go b/pkg/cli/add_interactive_orchestrator.go index 2cc9fc57e0f..d8645bf40ff 100644 --- a/pkg/cli/add_interactive_orchestrator.go +++ b/pkg/cli/add_interactive_orchestrator.go @@ -76,7 +76,7 @@ func RunAddInteractive(ctx context.Context, config *AddInteractiveConfig) error // are treated consistently across test and automation environments, while // IsRunningInCI centralizes the broader CI environment detection logic. if envutil.GetBoolFromEnv("GO_TEST_MODE", false, addInteractiveLog) || IsRunningInCI() { - return errors.New("interactive add is unavailable in automated tests or CI environments. Expected an interactive terminal. Example: unset CI and run the command from a terminal") + return errors.New("interactive add is unavailable in automated tests or CI environments. Expected an interactive terminal outside automation. Example: run `gh aw add-wizard` from a local terminal") } // Set context on the config diff --git a/pkg/cli/add_interactive_orchestrator_test.go b/pkg/cli/add_interactive_orchestrator_test.go index c36bcdd1583..b53e8f215b0 100644 --- a/pkg/cli/add_interactive_orchestrator_test.go +++ b/pkg/cli/add_interactive_orchestrator_test.go @@ -3,12 +3,33 @@ package cli import ( + "context" + "os" + "strings" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) +func TestRunAddInteractive_InAutomatedEnvironment(t *testing.T) { + origTestMode := os.Getenv("GO_TEST_MODE") + os.Setenv("GO_TEST_MODE", "true") + t.Cleanup(func() { + if origTestMode != "" { + os.Setenv("GO_TEST_MODE", origTestMode) + } else { + os.Unsetenv("GO_TEST_MODE") + } + }) + + err := RunAddInteractive(context.Background(), &AddInteractiveConfig{}) + require.Error(t, err) + if !strings.Contains(err.Error(), "Expected an interactive terminal outside automation") || !strings.Contains(err.Error(), "Example: run `gh aw add-wizard` from a local terminal") { + t.Errorf("Expected actionable error message, got %q", err.Error()) + } +} + func TestAddInteractiveConfig_determineFilesToAdd(t *testing.T) { t.Parallel() tests := []struct { diff --git a/pkg/cli/add_package_manifest_test.go b/pkg/cli/add_package_manifest_test.go index 0bc1e9369b5..825e55f2d44 100644 --- a/pkg/cli/add_package_manifest_test.go +++ b/pkg/cli/add_package_manifest_test.go @@ -846,6 +846,9 @@ func TestResolveWorkflows_RepositoryPackageRejectsPrivateTrue(t *testing.T) { _, err := ResolveWorkflows(context.Background(), []string{"owner/repo"}, false) require.Error(t, err) require.ErrorContains(t, err, `workflow "workflows/review.md" sets private: true`) + require.ErrorContains(t, err, "Expected an installable workflow with private: false") + require.ErrorContains(t, err, "Example: private: false") + require.NotContains(t, err.Error(), "\n") } func TestResolveWorkflows_NestedRepositoryPackage(t *testing.T) { diff --git a/pkg/cli/add_workflow_resolution.go b/pkg/cli/add_workflow_resolution.go index f340eda912c..7b054f8b7ce 100644 --- a/pkg/cli/add_workflow_resolution.go +++ b/pkg/cli/add_workflow_resolution.go @@ -260,7 +260,7 @@ func validateCurrentRepositorySpecs(parsedSpecs []*WorkflowSpec) error { continue } if spec.RepoSlug == currentRepoSlug { - return fmt.Errorf("workflow source %q is the current repository. Expected a workflow from another repository. Example: gh aw add github/gh-aw/example-workflow", currentRepoSlug) + return fmt.Errorf("cannot add workflows from the current repository %q. Expected a workflow from another repository. Example: gh aw add octo-org/agentic-workflows/example-workflow", currentRepoSlug) } } return nil @@ -408,9 +408,9 @@ func validateManifestWorkflowPrivateSetting(spec, resolvedSpec *WorkflowSpec, co } manifestPath := joinRepositoryPackagePath(spec.PackagePath, repositoryPackageManifestFileName) return fmt.Errorf( - "agentic workflow manifest %q sets workflow %q to private: true. Expected an installable workflow with private: false. Example:\nprivate: false", - manifestPath, + "workflow %q sets private: true in agentic workflow manifest %q. Expected an installable workflow with private: false. Example: private: false", resolvedSpec.WorkflowPath, + manifestPath, ) } diff --git a/pkg/cli/audit_diff_test.go b/pkg/cli/audit_diff_test.go index 03c26f64058..8bd04ee28fc 100644 --- a/pkg/cli/audit_diff_test.go +++ b/pkg/cli/audit_diff_test.go @@ -4,6 +4,7 @@ package cli import ( "encoding/json" + "strings" "testing" "time" @@ -11,6 +12,47 @@ import ( "github.com/stretchr/testify/require" ) +func TestAuditDiffValidationErrorsAreActionable(t *testing.T) { + tests := []struct { + name string + args []string + want string + }{ + { + name: "base run ID is non-numeric", + args: []string{"abc", "12346"}, + want: "Expected a numeric GitHub Actions run ID", + }, + { + name: "comparison run ID matches base", + args: []string{"12345", "12345"}, + want: "Expected a different run ID for comparison", + }, + { + name: "comparison run ID repeats", + args: []string{"12345", "12346", "12346"}, + want: "Expected each comparison run ID once", + }, + { + name: "repository is not owner repo", + args: []string{"12345", "12346", "--repo", "github"}, + want: "Expected an owner and repository name separated by '/'", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cmd := NewAuditDiffSubcommand() + cmd.SetArgs(tt.args) + err := cmd.Execute() + require.Error(t, err) + if !strings.Contains(err.Error(), tt.want) || !strings.Contains(err.Error(), "Example:") { + t.Errorf("Expected actionable error containing %q and Example:, got %q", tt.want, err.Error()) + } + }) + } +} + func TestComputeFirewallDiff_NewDomains(t *testing.T) { run1 := &FirewallAnalysis{ AnalysisBase: AnalysisBase{TotalRequests: 5, AllowedRequests: 5}, diff --git a/pkg/cli/interactive.go b/pkg/cli/interactive.go index 8ac7bb41131..0547419de50 100644 --- a/pkg/cli/interactive.go +++ b/pkg/cli/interactive.go @@ -69,7 +69,7 @@ func CreateWorkflowInteractively(ctx context.Context, workflowName string, verbo // are treated consistently across test and automation environments, while // IsRunningInCI centralizes the broader CI environment detection logic. if envutil.GetBoolFromEnv("GO_TEST_MODE", false, interactiveLog) || IsRunningInCI() { - return errors.New("interactive workflow creation is unavailable in automated tests or CI environments. Expected an interactive terminal. Example: unset CI and run the command from a terminal") + return errors.New("interactive workflow creation is unavailable in automated tests or CI environments. Expected an interactive terminal outside automation. Example: run `gh aw new` from a local terminal") } if verbose { @@ -411,7 +411,7 @@ func promptNonInteractiveSelect(scanner *bufio.Scanner, title string, options [] return opt.value, nil } } - return "", fmt.Errorf("selection %q is not available. Expected a displayed option name or number. Example: 1", input) + return "", fmt.Errorf("selection %q is not available. Expected a number or option value. Example: 1", input) } // promptNonInteractiveMultiSelect prints a numbered list and reads comma-separated selections. @@ -453,7 +453,7 @@ func promptNonInteractiveMultiSelect(scanner *bufio.Scanner, title string, optio // Try numeric index if idx, err := strconv.Atoi(tok); err == nil { if idx < 1 || idx > len(options) { - return nil, fmt.Errorf("selection %d is out of range. Expected a number from 1 to %d. Example: 1", idx, len(options)) + return nil, fmt.Errorf("selection %d is out of range. Expected a number from 1 to %d. Example: 1,2", idx, len(options)) } val := options[idx-1].value if _, dup := seen[val]; !dup { @@ -472,7 +472,7 @@ func promptNonInteractiveMultiSelect(scanner *bufio.Scanner, title string, optio continue } - return nil, fmt.Errorf("option %q is not available. Expected a displayed option name or number. Example: 1", tok) + return nil, fmt.Errorf("option %q is not available. Expected comma-separated numbers or option values. Example: 1,2", tok) } return selected, nil } diff --git a/pkg/cli/interactive_test.go b/pkg/cli/interactive_test.go index d7880089d4b..19dfc6a1570 100644 --- a/pkg/cli/interactive_test.go +++ b/pkg/cli/interactive_test.go @@ -442,7 +442,7 @@ func TestCreateWorkflowInteractively_InAutomatedEnvironment(t *testing.T) { if !strings.Contains(err.Error(), expectedErrMsg) { t.Errorf("Expected error containing %q, got %q", expectedErrMsg, err.Error()) } - if !strings.Contains(err.Error(), "Expected an interactive terminal") || !strings.Contains(err.Error(), "Example: unset CI") { + if !strings.Contains(err.Error(), "Expected an interactive terminal outside automation") || !strings.Contains(err.Error(), "Example: run `gh aw new` from a local terminal") { t.Errorf("Expected actionable error message, got %q", err.Error()) } } @@ -798,6 +798,9 @@ func TestPromptNonInteractiveSelect_InvalidValue(t *testing.T) { if err == nil { t.Fatal("expected error for unknown value, got nil") } + if !strings.Contains(err.Error(), "Expected a number or option value") || !strings.Contains(err.Error(), "Example: 1") { + t.Errorf("Expected actionable select error message, got %q", err.Error()) + } } func TestPromptNonInteractiveSelect_EOF(t *testing.T) { @@ -877,6 +880,9 @@ func TestPromptNonInteractiveMultiSelect_OutOfRange(t *testing.T) { if err == nil { t.Fatal("expected error for out-of-range index, got nil") } + if !strings.Contains(err.Error(), "Expected a number from 1 to 1") || !strings.Contains(err.Error(), "Example: 1,2") { + t.Errorf("Expected actionable multi-select range error message, got %q", err.Error()) + } } func TestPromptNonInteractiveMultiSelect_UnknownValue(t *testing.T) { @@ -886,6 +892,9 @@ func TestPromptNonInteractiveMultiSelect_UnknownValue(t *testing.T) { if err == nil { t.Fatal("expected error for unknown value, got nil") } + if !strings.Contains(err.Error(), "Expected comma-separated numbers or option values") || !strings.Contains(err.Error(), "Example: 1,2") { + t.Errorf("Expected actionable multi-select value error message, got %q", err.Error()) + } } func TestPromptForWorkflowNameFrom_Valid(t *testing.T) { diff --git a/pkg/cli/org_runner.go b/pkg/cli/org_runner.go index f3f7b359bf3..a4f2377e848 100644 --- a/pkg/cli/org_runner.go +++ b/pkg/cli/org_runner.go @@ -146,19 +146,19 @@ func runCommandForOrg(ctx context.Context, org string, repoGlobs []string, cbs o return errors.New("createPR and createIssue are mutually exclusive") } if cbs.SearchFn == nil { - return errors.New("organization search callback is not configured. Expected orgRunCallbacks.SearchFn to search organization repositories. Example: orgRunCallbacks{SearchFn: searchFn}") + return errors.New("organization search callback is not configured. Expected orgRunCallbacks.SearchFn to search organization repositories. Example: configure SearchFn before calling runCommandForOrg") } if cbs.ReportFn == nil { - return errors.New("organization report callback is not configured. Expected orgRunCallbacks.ReportFn to display the run summary. Example: orgRunCallbacks{ReportFn: reportFn}") + return errors.New("organization report callback is not configured. Expected orgRunCallbacks.ReportFn to display the run summary. Example: configure ReportFn before calling runCommandForOrg") } if createPR && cbs.ApplyFn == nil { - return errors.New("pull request callback is not configured. Expected orgRunCallbacks.ApplyFn when createPR is enabled. Example: orgRunCallbacks{ApplyFn: applyFn}") + return errors.New("pull request callback is not configured. Expected orgRunCallbacks.ApplyFn when createPR is enabled. Example: configure ApplyFn before calling runCommandForOrg with createPR") } if createIssue && cbs.IssueFn == nil { - return errors.New("issue callback is not configured. Expected orgRunCallbacks.IssueFn when createIssue is enabled. Example: orgRunCallbacks{IssueFn: issueFn}") + return errors.New("issue callback is not configured. Expected orgRunCallbacks.IssueFn when createIssue is enabled. Example: configure IssueFn before calling runCommandForOrg with createIssue") } if (createPR || createIssue) && !cbs.AutoYes && isRunningInCIFn() { - return errors.New("organization create operations in CI need confirmation. Expected --yes to auto-accept in non-interactive environments. Example: gh aw run --org octo-org --yes") + return errors.New("organization create operations in CI need confirmation. Expected --yes to auto-accept in non-interactive environments. Example: gh aw update --org octo-org --create-pull-request --yes") } // Handle Ctrl-C / SIGTERM so an interrupted run still renders the report diff --git a/pkg/cli/org_runner_test.go b/pkg/cli/org_runner_test.go new file mode 100644 index 00000000000..afbf4009ec8 --- /dev/null +++ b/pkg/cli/org_runner_test.go @@ -0,0 +1,82 @@ +//go:build !integration + +package cli + +import ( + "context" + "strings" + "testing" +) + +func TestRunCommandForOrgValidationErrorsAreActionable(t *testing.T) { + tests := []struct { + name string + callbacks orgRunCallbacks + createPR bool + createIssue bool + want string + }{ + { + name: "missing search callback", + want: "Expected orgRunCallbacks.SearchFn", + }, + { + name: "missing report callback", + callbacks: orgRunCallbacks{SearchFn: func(context.Context, string, bool) ([]string, error) { return nil, nil }}, + want: "Expected orgRunCallbacks.ReportFn", + }, + { + name: "missing apply callback", + createPR: true, + callbacks: orgRunCallbacks{ + SearchFn: func(context.Context, string, bool) ([]string, error) { return nil, nil }, + ReportFn: func([]orgRepoPreview, bool) {}, + }, + want: "Expected orgRunCallbacks.ApplyFn", + }, + { + name: "missing issue callback", + createIssue: true, + callbacks: orgRunCallbacks{ + SearchFn: func(context.Context, string, bool) ([]string, error) { return nil, nil }, + ReportFn: func([]orgRepoPreview, bool) {}, + }, + want: "Expected orgRunCallbacks.IssueFn", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := runCommandForOrg(context.Background(), "octo-org", nil, tt.callbacks, tt.createPR, tt.createIssue, false) + if err == nil { + t.Fatal("expected validation error, got nil") + } + if !strings.Contains(err.Error(), tt.want) || !strings.Contains(err.Error(), "Example:") { + t.Errorf("Expected actionable error containing %q and Example:, got %q", tt.want, err.Error()) + } + if strings.Contains(err.Error(), "orgRunCallbacks{") { + t.Errorf("Expected error not to expose raw struct literal syntax, got %q", err.Error()) + } + }) + } +} + +func TestRunCommandForOrgRequiresYesForCreateOperationsInCI(t *testing.T) { + origIsRunningInCI := isRunningInCIFn + isRunningInCIFn = func() bool { return true } + t.Cleanup(func() { isRunningInCIFn = origIsRunningInCI }) + + callbacks := orgRunCallbacks{ + SearchFn: func(context.Context, string, bool) ([]string, error) { return nil, nil }, + ReportFn: func([]orgRepoPreview, bool) {}, + ApplyFn: func(context.Context, orgRepoPreview, bool) error { return nil }, + } + + err := runCommandForOrg(context.Background(), "octo-org", nil, callbacks, true, false, false) + if err == nil { + t.Fatal("expected CI confirmation error, got nil") + } + if !strings.Contains(err.Error(), "Expected --yes") || !strings.Contains(err.Error(), "Example: gh aw update --org octo-org --create-pull-request --yes") { + t.Errorf("Expected actionable CI confirmation error, got %q", err.Error()) + } +} diff --git a/pkg/workflow/call_workflow_validation.go b/pkg/workflow/call_workflow_validation.go index da1a1674965..2198b9cf36f 100644 --- a/pkg/workflow/call_workflow_validation.go +++ b/pkg/workflow/call_workflow_validation.go @@ -117,12 +117,12 @@ func validateYAMLWorkflowHasCallTrigger(path, workflowName string) error { } onSection, hasOn := workflow["on"] if !hasOn { - return fmt.Errorf("call-workflow: workflow '%s' has no 'on' trigger section, expected an 'on' section with a 'workflow_call' trigger. Example:\non:\n workflow_call:", workflowName) + return fmt.Errorf("call-workflow: workflow '%s' has no 'on' trigger section, expected an 'on' section with a 'workflow_call' trigger. Example: on: workflow_call", workflowName) } if containsWorkflowCall(onSection) { return nil } - return fmt.Errorf("call-workflow: workflow '%s' does not support the workflow_call trigger, expected 'workflow_call' in the 'on' section. Example:\non:\n workflow_call:", workflowName) + return fmt.Errorf("call-workflow: workflow '%s' does not support the workflow_call trigger, expected 'workflow_call' in the 'on' section. Example: on: workflow_call", workflowName) } func validateMarkdownWorkflowHasCallTrigger(path, workflowName string) error { @@ -131,7 +131,7 @@ func validateMarkdownWorkflowHasCallTrigger(path, workflowName string) error { return fmt.Errorf("call-workflow: failed to read workflow source %s: %w", path, checkErr) } if !mdHasCall { - return fmt.Errorf("call-workflow: workflow '%s' does not support the workflow_call trigger, expected 'workflow_call' in the 'on' section. Example:\non:\n workflow_call:", workflowName) + return fmt.Errorf("call-workflow: workflow '%s' does not support the workflow_call trigger, expected 'workflow_call' in the 'on' section. Example: on: workflow_call", workflowName) } callWorkflowValidationLog.Printf("Workflow '%s' is valid for call-workflow (found .md source at %s with workflow_call trigger)", workflowName, path) return nil