Skip to content
Merged
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
261 changes: 261 additions & 0 deletions .github/instructions/error-messages.instructions.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,261 @@
---
description: Error Message Style Guide for Validation Errors
---

# Error Message Style Guide

This guide establishes the standard format for validation error messages in the gh-aw codebase. All validation errors should be clear, actionable, and include examples.

## Error Message Template

```
[what's wrong]. [what's expected]. [example of correct usage]
```

Each error message should answer three questions:
1. **What's wrong?** - Clearly state the validation error
2. **What's expected?** - Explain the valid format or values
3. **How to fix it?** - Provide a concrete example of correct usage

## Good Examples

These examples follow the template and provide actionable guidance:

### Time Delta Validation (from time_delta.go)
```go
return nil, fmt.Errorf("invalid time delta format: +%s. Expected format like +25h, +3d, +1w, +1mo, +1d12h30m", deltaStr)
```
✅ **Why it's good:**
- Clearly identifies the invalid input
- Lists multiple valid format examples
- Shows combined formats (+1d12h30m)

### Type Validation with Example
```go
return "", fmt.Errorf("manual-approval value must be a string, got %T. Example: manual-approval: \"production\"", val)
```
✅ **Why it's good:**
- Shows actual type received (%T)
- Provides concrete YAML example
- Uses proper YAML syntax with quotes

### Enum Validation with Options
```go
return fmt.Errorf("invalid engine: %s. Valid engines are: copilot, claude, codex, custom. Example: engine: copilot", engineID)
```
✅ **Why it's good:**
- Lists all valid options
- Provides simplest example
- Uses consistent formatting

### MCP Configuration
```go
return fmt.Errorf("tool '%s' mcp configuration must specify either 'command' or 'container'. Example:\ntools:\n %s:\n command: \"npx @my/tool\"", toolName, toolName)
```
✅ **Why it's good:**
- Explains mutual exclusivity
- Shows realistic tool name
- Formats multi-line YAML example

## Bad Examples

These examples lack clarity or actionable guidance:

### Too Vague
```go
return fmt.Errorf("invalid format")
```
❌ **Problems:**
- Doesn't specify what format is invalid
- Doesn't explain expected format
- No example provided

### Missing Example
```go
return fmt.Errorf("manual-approval value must be a string")
```
❌ **Problems:**
- States requirement but no example
- User doesn't know proper YAML syntax
- Could be clearer about type received

### Incomplete Information
```go
return fmt.Errorf("invalid engine: %s", engineID)
```
❌ **Problems:**
- Doesn't list valid options
- No guidance on fixing the error
- User must search documentation

## When to Include Examples

Always include examples for:

1. **Format/Syntax Errors** - Show the correct syntax
```go
fmt.Errorf("invalid date format. Expected: YYYY-MM-DD HH:MM:SS. Example: 2024-01-15 14:30:00")
```

2. **Enum/Choice Fields** - List all valid options
```go
fmt.Errorf("invalid permission level: %s. Valid levels: read, write, none. Example: permissions:\n contents: read", level)
```

3. **Type Mismatches** - Show expected type and example
```go
fmt.Errorf("timeout-minutes must be an integer, got %T. Example: timeout-minutes: 10", value)
```

4. **Complex Configurations** - Provide complete valid example
```go
fmt.Errorf("invalid MCP server config. Example:\nmcp-servers:\n my-server:\n command: \"node\"\n args: [\"server.js\"]")
```

## When Examples May Be Optional

Examples can be omitted when:

1. **Error is from wrapped error** - When wrapping another error with context
```go
return fmt.Errorf("failed to parse configuration: %w", err)
```

2. **Error is self-explanatory with clear context**
```go
return fmt.Errorf("duplicate unit '%s' in time delta: +%s", unit, deltaStr)
```

3. **Error points to specific documentation**
```go
return fmt.Errorf("unsupported feature. See https://docs.example.com/features")
```

## Formatting Guidelines

### Use Type Verbs for Dynamic Content
- `%s` - strings
- `%d` - integers
- `%T` - type of value
- `%v` - general value
- `%w` - wrapped errors

### Multi-line Examples
For YAML configuration examples spanning multiple lines:
```go
fmt.Errorf("invalid config. Example:\ntools:\n github:\n mode: \"remote\"")
```

### Quoting in Examples
Use proper YAML syntax in examples:
```go
// Good - shows quotes when needed
fmt.Errorf("Example: name: \"my-workflow\"")

// Good - shows no quotes for simple values
fmt.Errorf("Example: timeout-minutes: 10")
```

### Consistent Terminology
Use the same field names as in YAML:
```go
// Good - matches YAML field name
fmt.Errorf("timeout-minutes must be positive")

// Bad - uses different name
fmt.Errorf("timeout must be positive")
```

## Error Message Testing

All improved error messages should have corresponding tests:

```go
func TestErrorMessageQuality(t *testing.T) {
err := validateSomething(invalidInput)
require.Error(t, err)

// Error should explain what's wrong
assert.Contains(t, err.Error(), "invalid")

// Error should include expected format or values
assert.Contains(t, err.Error(), "Expected")

// Error should include example
assert.Contains(t, err.Error(), "Example:")
}
```

## Migration Strategy

When improving existing error messages:

1. **Identify the error** - Find validation error that lacks clarity
2. **Analyze context** - Understand what's being validated
3. **Apply template** - Add what's wrong + expected + example
4. **Add tests** - Verify error message content
5. **Update comments** - Document the validation logic

## Examples by Category

### Format Validation
```go
// Time deltas
fmt.Errorf("invalid time delta format: +%s. Expected format like +25h, +3d, +1w, +1mo, +1d12h30m", input)

// Dates
fmt.Errorf("invalid date format: %s. Expected: YYYY-MM-DD or relative like -1w. Example: 2024-01-15 or -7d", input)

// URLs
fmt.Errorf("invalid URL format: %s. Expected: https:// URL. Example: https://api.example.com", input)
```

### Type Validation
```go
// Boolean expected
fmt.Errorf("read-only must be a boolean, got %T. Example: read-only: true", value)

// String expected
fmt.Errorf("workflow name must be a string, got %T. Example: name: \"my-workflow\"", value)

// Object expected
fmt.Errorf("permissions must be an object, got %T. Example: permissions:\n contents: read", value)
```

### Choice/Enum Validation
```go
// Engine selection
fmt.Errorf("invalid engine: %s. Valid engines: copilot, claude, codex, custom. Example: engine: copilot", id)

// Permission levels
fmt.Errorf("invalid permission level: %s. Valid levels: read, write, none. Example: contents: read", level)

// Tool modes
fmt.Errorf("invalid mode: %s. Valid modes: local, remote. Example: mode: \"remote\"", mode)
```

### Configuration Validation
```go
// Missing required field
fmt.Errorf("tool '%s' missing required 'command' field. Example:\ntools:\n %s:\n command: \"node server.js\"", name, name)

// Mutually exclusive fields
fmt.Errorf("cannot specify both 'command' and 'container'. Choose one. Example: command: \"node server.js\"")

// Invalid combination
fmt.Errorf("http MCP servers cannot use 'container' field. Example:\ntools:\n my-http:\n type: http\n url: \"https://api.example.com\"")
```

## References

- **Excellent example to follow**: `pkg/workflow/time_delta.go`
- **Pattern inspiration**: Go standard library error messages
- **Testing examples**: `pkg/workflow/*_test.go`

## Tools

When writing error messages, consider:
- The user's perspective (what do they need to fix it?)
- The context (where in the workflow is the error?)
- The documentation (should we reference specific docs?)
- The complexity (is multi-line example needed?)
5 changes: 5 additions & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ require (
github.com/santhosh-tekuri/jsonschema/v6 v6.0.2
github.com/sourcegraph/conc v0.3.0
github.com/spf13/cobra v1.10.1
github.com/stretchr/testify v1.8.1
)

require (
Expand All @@ -33,11 +34,13 @@ require (
github.com/clipperhouse/displaywidth v0.5.0 // indirect
github.com/clipperhouse/stringish v0.1.1 // indirect
github.com/clipperhouse/uax29/v2 v2.3.0 // indirect
github.com/davecgh/go-spew v1.1.1 // indirect
github.com/dustin/go-humanize v1.0.1 // indirect
github.com/erikgeiser/coninput v0.0.0-20211004153227-1c3628e74d0f // indirect
github.com/fatih/color v1.7.0 // indirect
github.com/henvic/httpretty v0.1.4 // indirect
github.com/inconshreveable/mousetrap v1.1.0 // indirect
github.com/kr/text v0.2.0 // indirect
github.com/lucasb-eyer/go-colorful v1.3.0 // indirect
github.com/mattn/go-colorable v0.1.13 // indirect
github.com/mattn/go-localereader v0.0.1 // indirect
Expand All @@ -46,6 +49,7 @@ require (
github.com/muesli/ansi v0.0.0-20230316100256-276c6243b2f6 // indirect
github.com/muesli/cancelreader v0.2.2 // indirect
github.com/muesli/termenv v0.16.0 // indirect
github.com/pmezard/go-difflib v1.0.0 // indirect
github.com/rivo/uniseg v0.4.7 // indirect
github.com/spf13/pflag v1.0.10 // indirect
github.com/thlib/go-timezone-local v0.0.7 // indirect
Expand All @@ -56,4 +60,5 @@ require (
golang.org/x/sys v0.38.0 // indirect
golang.org/x/term v0.37.0 // indirect
golang.org/x/text v0.31.0 // indirect
gopkg.in/yaml.v3 v3.0.1 // indirect
)
14 changes: 14 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ github.com/clipperhouse/stringish v0.1.1/go.mod h1:v/WhFtE1q0ovMta2+m+UbpZ+2/HEX
github.com/clipperhouse/uax29/v2 v2.3.0 h1:SNdx9DVUqMoBuBoW3iLOj4FQv3dN5mDtuqwuhIGpJy4=
github.com/clipperhouse/uax29/v2 v2.3.0/go.mod h1:Wn1g7MK6OoeDT0vL+Q0SQLDz/KpfsVRgg6W7ihQeh4g=
github.com/cpuguy83/go-md2man/v2 v2.0.6/go.mod h1:oOW0eioCTA6cOiMLiUPZOpcVxMig6NIQQ7OS05n1F4g=
github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E=
github.com/creack/pty v1.1.24 h1:bJrF4RRfyJnbTJqzRLHzcGaZK1NeM5kTC9jGgovnR1s=
github.com/creack/pty v1.1.24/go.mod h1:08sCNb52WyoAwi2QDyzUCTgcvVFhUzewun7wtTfvcwE=
github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
Expand All @@ -76,6 +77,10 @@ github.com/henvic/httpretty v0.1.4 h1:Jo7uwIRWVFxkqOnErcoYfH90o3ddQyVrSANeS4cxYm
github.com/henvic/httpretty v0.1.4/go.mod h1:Dn60sQTZfbt2dYsdUSNsCljyF4AfdqnuJFDLJA1I4AM=
github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2s0bqwp9tc8=
github.com/inconshreveable/mousetrap v1.1.0/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLfsEA9PFc4w1p2J65bw=
github.com/kr/pretty v0.3.0 h1:WgNl7dwNpEZ6jJ9k1snq4pZsg7DOEN8hP9Xw0Tsjwk0=
github.com/kr/pretty v0.3.0/go.mod h1:640gp4NfQd8pI5XOwp5fnNeVWj67G7CFk/SaSQn7NBk=
github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY=
github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE=
github.com/lucasb-eyer/go-colorful v1.3.0 h1:2/yBRLdWBZKrf7gB40FoiKfAWYQ0lqNcbuQwVHXptag=
github.com/lucasb-eyer/go-colorful v1.3.0/go.mod h1:R4dSotOR9KMtayYi1e77YzuveK+i7ruzyGqttikkLy0=
github.com/mattn/go-colorable v0.1.13 h1:fFA4WZxdEF4tXPZVKMLwD8oUnCTTo08duU7wxecdEvA=
Expand Down Expand Up @@ -103,6 +108,8 @@ github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZb
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
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.9.0 h1:73kH8U+JUqXU8lRuOHeVHaa/SZPifC7BkcraZVejAe8=
github.com/rogpeppe/go-internal v1.9.0/go.mod h1:WtVeX8xhTBvf0smdhujwtBcq4Qrzq/fJaraNFVN+nFs=
github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM=
github.com/santhosh-tekuri/jsonschema/v6 v6.0.2 h1:KRzFb2m7YtdldCEkzs6KqmJw4nqEVZGK7IN2kJkjTuQ=
github.com/santhosh-tekuri/jsonschema/v6 v6.0.2/go.mod h1:JXeL+ps8p7/KNMjDQk3TCwPpBy0wYklyWTfbkIzdIFU=
Expand All @@ -114,7 +121,11 @@ github.com/spf13/pflag v1.0.9/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An
github.com/spf13/pflag v1.0.10 h1:4EBh2KAYBwaONj6b2Ye1GiHfwjqyROoF4RwYO+vPwFk=
github.com/spf13/pflag v1.0.10/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg=
github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw=
github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo=
github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=
github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU=
github.com/stretchr/testify v1.8.1 h1:w7B6lhMri9wdJUVmEZPGGhZzrYTPvgJArz7wNPgYKsk=
github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4=
github.com/thlib/go-timezone-local v0.0.7 h1:fX8zd3aJydqLlTs/TrROrIIdztzsdFV23OzOQx31jII=
Expand Down Expand Up @@ -142,5 +153,8 @@ golang.org/x/text v0.31.0/go.mod h1:tKRAlv61yKIjGGHX/4tP1LTbc13YSec1pxVEWXzfoeM=
golang.org/x/tools v0.38.0 h1:Hx2Xv8hISq8Lm16jvBZ2VQf+RLmbd7wVUsALibYI/IQ=
golang.org/x/tools v0.38.0/go.mod h1:yEsQ/d/YK8cjh0L6rZlY8tgtlKiBNTL14pGDJPJpYQs=
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15 h1:YR8cESwS4TdDjEe65xsg0ogRM/Nc3DYOhEAlW+xobZo=
gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
4 changes: 2 additions & 2 deletions pkg/workflow/agent_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ func (c *Compiler) validateHTTPTransportSupport(tools map[string]any, engine Cod
for toolName, toolConfig := range tools {
if config, ok := toolConfig.(map[string]any); ok {
if hasMcp, mcpType := hasMCPConfig(config); hasMcp && mcpType == "http" {
return fmt.Errorf("tool '%s' uses HTTP transport which is not supported by engine '%s' (only stdio transport is supported)", toolName, engine.GetID())
return fmt.Errorf("tool '%s' uses HTTP transport which is not supported by engine '%s'. Only stdio transport is supported. Use a different engine (e.g., copilot) or change the tool to use stdio transport. Example:\ntools:\n %s:\n type: stdio\n command: \"node server.js\"", toolName, engine.GetID(), toolName)
}
}
}
Expand All @@ -149,7 +149,7 @@ func (c *Compiler) validateMaxTurnsSupport(frontmatter map[string]any, engine Co

// max-turns is specified, check if the engine supports it
if !engine.SupportsMaxTurns() {
return fmt.Errorf("max-turns not supported: engine '%s' does not support the max-turns feature", engine.GetID())
return fmt.Errorf("max-turns not supported: engine '%s' does not support the max-turns feature. Use engine: copilot or remove max-turns from your configuration. Example:\nengine:\n id: copilot\n max-turns: 5", engine.GetID())
}

// Engine supports max-turns - additional validation could be added here if needed
Expand Down
2 changes: 1 addition & 1 deletion pkg/workflow/docker_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ func validateDockerImage(image string, verbose bool) error {
}

// Other errors indicate the image truly doesn't exist or has issues
return fmt.Errorf("container image '%s' not found and could not be pulled: %s", image, outputStr)
return fmt.Errorf("container image '%s' not found and could not be pulled: %s. Please verify the image name and tag. Example: container: \"node:20\" or container: \"ghcr.io/owner/image:latest\"", image, outputStr)
}

// Successfully pulled
Expand Down
Loading
Loading