Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cmd/gh-aw/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 12 additions & 0 deletions cmd/gh-aw/main_entry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{})
})
}
16 changes: 12 additions & 4 deletions pkg/cli/logs_command.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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) {
Expand Down Expand Up @@ -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
}
29 changes: 29 additions & 0 deletions pkg/cli/logs_command_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:")
}
9 changes: 4 additions & 5 deletions pkg/cli/outcome_eval.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package cli
import (
"context"
"encoding/json"
"errors"
"fmt"
"maps"
"net/url"
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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!) {
Expand Down
2 changes: 2 additions & 0 deletions pkg/cli/outcome_eval_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:")
})
}
}
Expand Down
8 changes: 4 additions & 4 deletions pkg/cli/run_interactive.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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: {}")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/codebase-design] The no runnable workflows error embeds a raw newline and YAML snippet in a errors.New string, which breaks single-line log/error formatting and makes programmatic matching harder.

💡 Suggestion

Keep the example concise without embedded newlines, or move multi-line YAML to a separate line using a const:

return errors.New("no runnable workflows were found. Expected at least one workflow with \"on: workflow_dispatch\". Example: on:\n  workflow_dispatch: {}")

Note this is consistent with dispatch_workflow_validation.go (compiler output), but interactive CLI errors surfaced via huh or a terminal prompt often do better without embedded block literals.

@copilot please address this.

}

// Step 2: Let user select a workflow
Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -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)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/codebase-design] The required-input validation error repeats inputName twice in the format string (input '%s' is required ... enter a value for '%s' such as my-value). The second repetition adds noise without new information and makes the message longer than it needs to be in a tight interactive prompt.

💡 Suggestion
return fmt.Errorf("input '%s' is required. Expected a non-empty value. Example: my-value", inputName)

This stays within the style guide while keeping the prompt-facing message concise.

@copilot please address this.

}
return nil
})
Expand Down
10 changes: 10 additions & 0 deletions pkg/cli/run_interactive_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
package cli

import (
"context"
"os"
"path/filepath"
"strings"
Expand Down Expand Up @@ -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:")
}
6 changes: 3 additions & 3 deletions pkg/workflow/call_workflow_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
Expand Down
4 changes: 4 additions & 0 deletions pkg/workflow/dispatch_repository_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
8 changes: 4 additions & 4 deletions pkg/workflow/dispatch_repository_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
}
Expand All @@ -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
}
Expand All @@ -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
}
Expand Down
8 changes: 4 additions & 4 deletions pkg/workflow/dispatch_workflow_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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
}
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/workflow/dispatch_workflow_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down