diff --git a/cmd/gh-aw/main.go b/cmd/gh-aw/main.go index ef7ac950f21..6cc3395d286 100644 --- a/cmd/gh-aw/main.go +++ b/cmd/gh-aw/main.go @@ -338,7 +338,7 @@ This command only works with workflows that have workflow_dispatch triggers. if len(args) == 0 { // Check if running in CI environment if cli.IsRunningInCI() { - return errors.New("interactive mode cannot be used in CI environments. Please provide a workflow name") + return errors.New("interactive mode is unavailable in CI environments. Expected a workflow name argument when running in CI. Example: gh aw run daily-perf-improver") } // Interactive mode doesn't support repeat or enable flags diff --git a/cmd/gh-aw/main_entry_test.go b/cmd/gh-aw/main_entry_test.go index d491c96f828..80f1452098c 100644 --- a/cmd/gh-aw/main_entry_test.go +++ b/cmd/gh-aw/main_entry_test.go @@ -451,4 +451,16 @@ func TestCommandErrorHandling(t *testing.T) { // Reset args for other tests rootCmd.SetArgs([]string{}) }) + + t.Run("run without arguments in CI produces actionable error", func(t *testing.T) { + t.Setenv("CI", "true") + rootCmd.SetArgs([]string{"run"}) + err := rootCmd.Execute() + + require.Error(t, err, "run without a workflow name in CI should fail") + assert.Contains(t, err.Error(), "Expected a workflow name argument when running in CI") + assert.Contains(t, err.Error(), "Example: gh aw run") + + rootCmd.SetArgs([]string{}) + }) } diff --git a/pkg/cli/logs_command.go b/pkg/cli/logs_command.go index a76600f433d..6501ccf8089 100644 --- a/pkg/cli/logs_command.go +++ b/pkg/cli/logs_command.go @@ -323,7 +323,11 @@ func validateLogsRuntime(runtime string) error { if slices.Contains(validRuntimes, runtime) { return nil } - return fmt.Errorf("invalid runtime value '%s'. Must be one of: %s", runtime, strings.Join(validRuntimes, ", ")) + exampleRuntime := "gvisor" + if len(validRuntimes) > 0 { + exampleRuntime = validRuntimes[0] + } + return fmt.Errorf("invalid runtime value %q. Expected one of: %s. Example: --runtime %s", runtime, strings.Join(validRuntimes, ", "), exampleRuntime) } func validateLogsEngine(engine string) error { @@ -336,7 +340,11 @@ func validateLogsEngine(engine string) error { return nil } supportedEngines := registry.GetSupportedEngines() - return fmt.Errorf("invalid engine value '%s'. Must be one of: %s", engine, strings.Join(supportedEngines, ", ")) + exampleEngine := "copilot" + if len(supportedEngines) > 0 { + exampleEngine = supportedEngines[0] + } + return fmt.Errorf("invalid engine value %q. Expected one of: %s. Example: --engine %s", engine, strings.Join(supportedEngines, ", "), exampleEngine) } func resolveLogsWorkflowName(cmd *cobra.Command, args []string) (string, error) { @@ -532,10 +540,10 @@ func validateReportFileFlags(reportFile, format string, jsonOutput bool) error { return nil } if format != "markdown" { - return errors.New("--report-file requires --format markdown") + return errors.New("--report-file was provided with a non-markdown format. Expected '--format markdown' when using '--report-file'. Example: gh aw logs --format markdown --report-file report.md") } if jsonOutput { - return errors.New("--report-file cannot be used with --json") + return errors.New("--report-file cannot be combined with --json output. Expected markdown output when writing a report file. Example: gh aw logs --format markdown --report-file report.md") } return nil } diff --git a/pkg/cli/logs_command_test.go b/pkg/cli/logs_command_test.go index 9bcab5fda6e..3aa2c3fabda 100644 --- a/pkg/cli/logs_command_test.go +++ b/pkg/cli/logs_command_test.go @@ -409,3 +409,32 @@ func TestLogsCommand_RepoUsesLocalResolutionWhenLockFileExists(t *testing.T) { assert.NotContains(t, execErr.Error(), "could not find any workflows named my-test-workflow", "when a local lock file exists, the display name (not the workflow ID) should be passed to gh run list") } + +func TestValidateLogsRuntimeErrorMessage(t *testing.T) { + err := validateLogsRuntime("not-a-real-runtime") + require.Error(t, err) + require.ErrorContains(t, err, "invalid runtime value") + require.ErrorContains(t, err, "Expected one of:") + require.ErrorContains(t, err, "Example: --runtime") +} + +func TestValidateLogsEngineErrorMessage(t *testing.T) { + err := validateLogsEngine("not-a-real-engine") + require.Error(t, err) + require.ErrorContains(t, err, "invalid engine value") + require.ErrorContains(t, err, "Expected one of:") + require.ErrorContains(t, err, "Example: --engine") +} + +func TestValidateReportFileFlagsErrorMessages(t *testing.T) { + err := validateReportFileFlags("report.md", "json", false) + require.Error(t, err) + require.ErrorContains(t, err, "Expected '--format markdown'") + require.ErrorContains(t, err, "Example:") + + err = validateReportFileFlags("report.md", "markdown", true) + require.Error(t, err) + require.ErrorContains(t, err, "cannot be combined with --json") + require.ErrorContains(t, err, "Expected markdown output") + require.ErrorContains(t, err, "Example:") +} diff --git a/pkg/cli/outcome_eval.go b/pkg/cli/outcome_eval.go index f6f79597c3b..71e5f179168 100644 --- a/pkg/cli/outcome_eval.go +++ b/pkg/cli/outcome_eval.go @@ -3,7 +3,6 @@ package cli import ( "context" "encoding/json" - "errors" "fmt" "maps" "net/url" @@ -241,10 +240,10 @@ func escapeOwnerRepo(ownerRepo string) string { func validateAPIEndpoint(endpoint string) error { if strings.HasPrefix(endpoint, "/") { - return errors.New("endpoint must not start with '/'") + return fmt.Errorf("endpoint %q must not start with '/'. Expected a relative API path without a leading slash. Example: issues/comments/123", endpoint) } if slices.Contains(strings.Split(endpoint, "/"), "..") { - return errors.New("endpoint must not contain '..' path segments") + return fmt.Errorf("endpoint %q must not contain '..' path segments. Expected a normalized API path without parent directory traversal. Example: issues/comments/123", endpoint) } return nil } @@ -326,7 +325,7 @@ func buildGraphQLArgs(query string, variables map[string]any) ([]string, error) case int, int32, int64, bool: args = append(args, "-F", fmt.Sprintf("%s=%v", name, value)) default: - return nil, fmt.Errorf("buildGraphQLArgs: unsupported variable type %T for key %q", value, name) + return nil, fmt.Errorf("GraphQL variable %q has unsupported type %T. Expected string, int, int32, int64, or bool. Example: map[string]any{\"number\": 42}", name, value) } } return args, nil @@ -515,7 +514,7 @@ func loadPullRequestIntentData(ctx context.Context, report OutcomeReport, repo s ownerRepo, _ := repoutil.NormalizeRepoForAPI(repo) owner, name, found := strings.Cut(ownerRepo, "/") if !found || owner == "" || name == "" { - return intent.PullRequestData{}, fmt.Errorf("invalid repo for root tracing: %s", repo) + return intent.PullRequestData{}, fmt.Errorf("repo value %q is not valid for root tracing. Expected 'owner/repo'. Example: github/gh-aw", repo) } query := `query($owner: String!, $name: String!, $number: Int!) { diff --git a/pkg/cli/outcome_eval_test.go b/pkg/cli/outcome_eval_test.go index d3e70d7e0ff..7d293454940 100644 --- a/pkg/cli/outcome_eval_test.go +++ b/pkg/cli/outcome_eval_test.go @@ -163,6 +163,8 @@ func TestValidateAPIEndpoint(t *testing.T) { } require.Error(t, err) require.ErrorContains(t, err, tt.wantErr) + require.ErrorContains(t, err, "Expected") + require.ErrorContains(t, err, "Example:") }) } } diff --git a/pkg/cli/run_interactive.go b/pkg/cli/run_interactive.go index f4e7006bb68..69aa0db182a 100644 --- a/pkg/cli/run_interactive.go +++ b/pkg/cli/run_interactive.go @@ -33,7 +33,7 @@ func RunWorkflowInteractively(ctx context.Context, opts RunWorkflowOptions) erro // Check if running in CI environment if IsRunningInCI() { - return errors.New("interactive mode cannot be used in CI environments") + return errors.New("interactive mode is unavailable in CI environments. Expected an interactive terminal session outside CI, or a workflow name argument. Example: gh aw run daily-perf-improver") } if opts.Verbose { @@ -47,7 +47,7 @@ func RunWorkflowInteractively(ctx context.Context, opts RunWorkflowOptions) erro } if len(workflows) == 0 { - return errors.New("no runnable workflows found. Workflows must have 'workflow_dispatch' trigger") + return errors.New("no runnable workflows were found. Expected at least one workflow with 'on: workflow_dispatch'. Example:\non:\n workflow_dispatch: {}") } // Step 2: Let user select a workflow @@ -221,7 +221,7 @@ func selectWorkflowNonInteractive(workflows []WorkflowOption) (*WorkflowOption, } if choice < 1 || choice > len(workflows) { - return nil, fmt.Errorf("selection out of range (must be 1-%d)", len(workflows)) + return nil, fmt.Errorf("selection %d is out of range. Expected a number between 1 and %d. Example: enter 1 to select the first workflow", choice, len(workflows)) } selectedWorkflow := &workflows[choice-1] @@ -301,7 +301,7 @@ func collectInputsWithMap(ctx context.Context, inputs map[string]*workflow.Input if inputDef.Required { field = field.Validate(func(s string) error { if s == "" { - return errors.New("this input is required") + return fmt.Errorf("input '%s' is required. Expected a non-empty value in the interactive prompt. Example: enter a value for '%s' such as my-value", inputName, inputName) } return nil }) diff --git a/pkg/cli/run_interactive_test.go b/pkg/cli/run_interactive_test.go index f73aa22c662..b4261b0569f 100644 --- a/pkg/cli/run_interactive_test.go +++ b/pkg/cli/run_interactive_test.go @@ -3,6 +3,7 @@ package cli import ( + "context" "os" "path/filepath" "strings" @@ -484,3 +485,12 @@ func TestSelectWorkflowNonInteractive(t *testing.T) { assert.NotEmpty(t, wf.Name, "Workflow at index %d should have a name", i) } } + +func TestRunWorkflowInteractively_CIErrorMessage(t *testing.T) { + t.Setenv("CI", "true") + err := RunWorkflowInteractively(context.Background(), RunWorkflowOptions{}) + require.Error(t, err) + require.ErrorContains(t, err, "interactive mode is unavailable in CI environments") + require.ErrorContains(t, err, "Expected an interactive terminal session outside CI") + require.ErrorContains(t, err, "Example:") +} diff --git a/pkg/workflow/call_workflow_validation.go b/pkg/workflow/call_workflow_validation.go index da1a1674965..b66edcf7ecc 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:\non:\n 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:\non:\n 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:\non:\n 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 diff --git a/pkg/workflow/dispatch_repository_test.go b/pkg/workflow/dispatch_repository_test.go index 5c198a3ab75..acbc97760ad 100644 --- a/pkg/workflow/dispatch_repository_test.go +++ b/pkg/workflow/dispatch_repository_test.go @@ -359,6 +359,8 @@ func TestValidateDispatchRepository_InvalidRepoFormat(t *testing.T) { err = compiler.validateDispatchRepository(workflowData, workflowPath) require.Error(t, err, "Validation should fail for invalid repository format") require.ErrorContains(t, err, "invalid", "Error should mention invalid format") + require.ErrorContains(t, err, "Expected 'owner/repo'", "Error should describe expected format") + require.ErrorContains(t, err, "Example:", "Error should include an example") } // TestValidateDispatchRepository_GitHubExpression tests that GitHub Actions expressions are accepted @@ -448,6 +450,8 @@ func TestValidateDispatchRepository_EmptyTools(t *testing.T) { err = compiler.validateDispatchRepository(workflowData, workflowPath) require.Error(t, err, "Validation should fail with empty tools map") require.ErrorContains(t, err, "at least one dispatch tool", "Error should mention tools requirement") + require.ErrorContains(t, err, "Expected", "Error should describe expected configuration") + require.ErrorContains(t, err, "Example:", "Error should include an example") } // TestValidateDispatchRepository_NilConfig tests that nil config is OK (no-op) diff --git a/pkg/workflow/dispatch_repository_validation.go b/pkg/workflow/dispatch_repository_validation.go index 63b9d9ff9d1..c7d6bcfb996 100644 --- a/pkg/workflow/dispatch_repository_validation.go +++ b/pkg/workflow/dispatch_repository_validation.go @@ -28,7 +28,7 @@ func (c *Compiler) validateDispatchRepository(data *WorkflowData, workflowPath s config := data.SafeOutputs.DispatchRepository if len(config.Tools) == 0 { - return errors.New("dispatch_repository: must specify at least one dispatch tool\n\nExample configuration in workflow frontmatter:\nsafe-outputs:\n dispatch_repository:\n trigger_ci:\n description: Trigger CI in another repository\n workflow: ci.yml\n event_type: ci_trigger\n repository: org/target-repo") + return errors.New("dispatch_repository configuration has no tools and must specify at least one dispatch tool. Expected at least one tool under safe-outputs.dispatch_repository. Example:\nsafe-outputs:\n dispatch_repository:\n trigger_ci:\n description: Trigger CI in another repository\n workflow: ci.yml\n event_type: ci_trigger\n repository: org/target-repo") } collector := NewErrorCollector(c.failFast) @@ -59,7 +59,7 @@ func (c *Compiler) validateDispatchRepository(data *WorkflowData, workflowPath s hasAllowedRepos := len(tool.AllowedRepositories) > 0 if !hasRepository && !hasAllowedRepos { - repoErr := fmt.Errorf("dispatch_repository: tool %q must specify either 'repository' or 'allowed_repositories'\n\nExample with single repository:\n dispatch_repository:\n %s:\n workflow: %s\n event_type: %s\n repository: org/target-repo\n\nExample with multiple repositories:\n dispatch_repository:\n %s:\n workflow: %s\n event_type: %s\n allowed_repositories:\n - org/repo1\n - org/repo2", toolKey, toolKey, tool.Workflow, tool.EventType, toolKey, tool.Workflow, tool.EventType) + repoErr := fmt.Errorf("dispatch_repository tool %q has no repository target. Expected either 'repository' or 'allowed_repositories'. Example:\n dispatch_repository:\n %s:\n workflow: %s\n event_type: %s\n repository: org/target-repo\n\nOr, to target multiple repositories:\n dispatch_repository:\n %s:\n workflow: %s\n event_type: %s\n allowed_repositories:\n - org/repo1\n - org/repo2", toolKey, toolKey, tool.Workflow, tool.EventType, toolKey, tool.Workflow, tool.EventType) if returnErr := collector.Add(repoErr); returnErr != nil { return returnErr } @@ -69,7 +69,7 @@ func (c *Compiler) validateDispatchRepository(data *WorkflowData, workflowPath s // Validate single repository format (skip if it looks like a GitHub Actions expression) if hasRepository && !hasExpressionMarker(tool.Repository) { if !repoSlugPattern.MatchString(tool.Repository) { - repoFmtErr := fmt.Errorf("dispatch_repository: tool %q has invalid 'repository' format %q (expected 'owner/repo')", toolKey, tool.Repository) + repoFmtErr := fmt.Errorf("dispatch_repository tool %q has invalid repository value %q in an unsupported format. Expected 'owner/repo'. Example: repository: github/gh-aw", toolKey, tool.Repository) if returnErr := collector.Add(repoFmtErr); returnErr != nil { return returnErr } @@ -86,7 +86,7 @@ func (c *Compiler) validateDispatchRepository(data *WorkflowData, workflowPath s continue } if !repoSlugPattern.MatchString(repo) { - allowedRepoErr := fmt.Errorf("dispatch_repository: tool %q has invalid repository %q in 'allowed_repositories' (expected 'owner/repo' format)", toolKey, repo) + allowedRepoErr := fmt.Errorf("dispatch_repository tool %q has allowed_repositories entry %q in an unsupported format. Expected entries like 'owner/repo'. Example:\nallowed_repositories:\n - github/gh-aw\n - octo-org/shared-service", toolKey, repo) if returnErr := collector.Add(allowedRepoErr); returnErr != nil { return returnErr } diff --git a/pkg/workflow/dispatch_workflow_validation.go b/pkg/workflow/dispatch_workflow_validation.go index 7c5c4f27dd2..25765392dca 100644 --- a/pkg/workflow/dispatch_workflow_validation.go +++ b/pkg/workflow/dispatch_workflow_validation.go @@ -27,7 +27,7 @@ func (c *Compiler) validateDispatchWorkflow(data *WorkflowData, workflowPath str config := data.SafeOutputs.DispatchWorkflow if len(config.Workflows) == 0 { - return errors.New("dispatch-workflow: must specify at least one workflow in the list\n\nExample configuration in workflow frontmatter:\nsafe-outputs:\n dispatch-workflow:\n workflows: [workflow-name-1, workflow-name-2]\n\nWorkflow names should match the filename without the .md extension") + return errors.New("dispatch-workflow configuration has no workflows and must specify at least one workflow in the list. Expected workflow names that match the filename without the .md extension. Example:\nsafe-outputs:\n dispatch-workflow:\n workflows: [workflow-name-1, workflow-name-2]") } if c.shouldSkipLocalDispatchWorkflowValidation(config.TargetRepoSlug) { @@ -42,7 +42,7 @@ func (c *Compiler) validateDispatchWorkflow(data *WorkflowData, workflowPath str for _, workflowName := range config.Workflows { dispatchWorkflowValidationLog.Printf("Validating workflow: %s", workflowName) if workflowName == currentWorkflowName { - selfRefErr := fmt.Errorf("dispatch-workflow: self-reference not allowed (workflow '%s' cannot dispatch itself)\n\nA workflow cannot trigger itself to prevent infinite loops.\nIf you need recurring execution, use a schedule trigger or workflow_dispatch instead", workflowName) + selfRefErr := fmt.Errorf("dispatch-workflow self-reference not allowed: workflow '%s' cannot dispatch itself and can create infinite loops. Expected each listed workflow to be different; use a schedule trigger or workflow_dispatch for recurring runs. Example:\nsafe-outputs:\n dispatch-workflow:\n workflows: [build, deploy]", workflowName) if returnErr := collector.Add(selfRefErr); returnErr != nil { return returnErr } @@ -103,7 +103,7 @@ func (c *Compiler) validateDispatchWorkflow(data *WorkflowData, workflowPath str continue } if !mdHasDispatch { - dispatchErr := fmt.Errorf("dispatch-workflow: workflow '%s' does not support workflow_dispatch trigger (must include 'workflow_dispatch' in the 'on' section)", workflowName) + dispatchErr := fmt.Errorf("dispatch-workflow target '%s' does not support workflow_dispatch trigger. Expected the target workflow to include 'on: workflow_dispatch'. Example:\non:\n workflow_dispatch: {}", workflowName) if returnErr := collector.Add(dispatchErr); returnErr != nil { return returnErr } @@ -132,7 +132,7 @@ func (c *Compiler) validateDispatchWorkflow(data *WorkflowData, workflowPath str } if !containsWorkflowDispatch(onSection) { - dispatchErr := fmt.Errorf("dispatch-workflow: workflow '%s' does not support workflow_dispatch trigger (must include 'workflow_dispatch' in the 'on' section)", workflowName) + dispatchErr := fmt.Errorf("dispatch-workflow target '%s' does not support workflow_dispatch trigger. Expected the target workflow to include 'on: workflow_dispatch'. Example:\non:\n workflow_dispatch: {}", workflowName) if returnErr := collector.Add(dispatchErr); returnErr != nil { return returnErr } diff --git a/pkg/workflow/dispatch_workflow_validation_test.go b/pkg/workflow/dispatch_workflow_validation_test.go index 969a3f0cd20..0492f75c00d 100644 --- a/pkg/workflow/dispatch_workflow_validation_test.go +++ b/pkg/workflow/dispatch_workflow_validation_test.go @@ -46,7 +46,7 @@ func TestDispatchWorkflowErrorMessage_EmptyList(t *testing.T) { // Verify enhanced error message content errMsg := err.Error() assert.Contains(t, errMsg, "must specify at least one workflow", "Should mention the requirement") - assert.Contains(t, errMsg, "Example configuration", "Should include example header") + assert.Contains(t, errMsg, "Example:", "Should include explicit example marker") assert.Contains(t, errMsg, "safe-outputs:", "Should show YAML structure") assert.Contains(t, errMsg, "dispatch-workflow:", "Should show feature name") assert.Contains(t, errMsg, "workflows: [workflow-name-1, workflow-name-2]", "Should show example list")