-
Notifications
You must be signed in to change notification settings - Fork 28
Closed
Labels
Description
Objective
Create and enforce a standard format for validation error messages across the codebase, following the excellent pattern established in time_delta.go.
Context
Part of Discussion #3956 - Workflow Validation and Error Feedback Quality improvements.
All validation errors should include: (1) what's wrong, (2) what was expected/valid format, (3) example of correct usage when applicable.
Implementation Approach
1. Create Error Message Style Guide
File: .github/instructions/error-messages.instructions.md
Content structure:
# Error Message Style Guide
## Template
"[what's wrong]. [what's expected]. [example of correct usage]"
## Good Examples
- "invalid time delta format: +%s. Expected format like +25h, +3d, +1w, +1mo, +1d12h30m"
- "manual-approval value must be a string, got %T. Example: manual-approval: \"production\""
## Bad Examples
- "invalid format" (lacks specifics)
- "manual-approval value must be a string" (no example)
## When to Include Examples
- Always for format/syntax errors
- Always for enum/choice fields
- When expected value format is not obvious
- When there are multiple valid formats2. Refactor High-Impact Error Messages
Target these files with at least 5 improvements:
pkg/workflow/manual_approval.go
// Before:
return "", fmt.Errorf("manual-approval value must be a string")
// After:
return "", fmt.Errorf("manual-approval value must be a string, got %T. Example: manual-approval: \"production\"", val)pkg/workflow/engine_validation.go
// Before:
return fmt.Errorf("invalid engine: %s", engineID)
// After:
return fmt.Errorf("invalid engine: %s. Valid engines are: copilot, claude, codex, custom. Example: engine: copilot", engineID)Other priority files:
pkg/workflow/mcp_config_validation.gopkg/workflow/permissions_validator.gopkg/workflow/docker_validation.go
3. Add Error Message Content Tests
Create tests that validate error message quality:
func TestErrorMessageFormat(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 example
assert.Contains(t, err.Error(), "Example:")
}Files to Create/Modify
- Create:
.github/instructions/error-messages.instructions.md - Update:
pkg/workflow/manual_approval.go - Update:
pkg/workflow/engine_validation.go - Update:
pkg/workflow/mcp_config_validation.go - Update:
pkg/workflow/permissions_validator.go - Update:
pkg/workflow/docker_validation.go - Update: Corresponding test files with error message validation
Acceptance Criteria
- Error message style guide created in
.github/instructions/ - Identified 20+ error messages that lack examples/guidance
- Refactored error messages to include format examples
- Added validation tests that check error message content
- Updated at least 5 high-impact error messages
- Documented before/after examples in commit message
- All tests pass (
make test-unit)
Priority
Medium - Systematic improvement that benefits all users
Related to #3956
AI generated by Plan Command for discussion #3956
Copilot