Standardize validation error messages with format examples

[Copilot]

Error Message Standardization - Complete ✅

Successfully standardized validation error messages across the codebase following the excellent pattern from time_delta.go.

Summary

Created:

1. Error Message Style Guide (.github/instructions/error-messages.instructions.md)

   • Comprehensive guide with template, examples, and best practices
   • Categories: format validation, type validation, choice/enum validation, configuration validation
   • When to include examples and when they're optional
   • Error message testing guidelines

2. Error Message Quality Tests (pkg/workflow/error_message_quality_test.go)

   • 11 comprehensive test cases
   • Validates error messages contain: what's wrong, what's expected, examples
   • Checks for vague error messages
   • Tests across multiple validation scenarios

Improved 18 Error Messages Across 6 Files:

• ✅ manual_approval.go - 2 messages
• ✅ engine_validation.go - 4 messages
• ✅ mcp_config_validation.go - 8 messages
• ✅ docker_validation.go - 1 message
• ✅ runtime_validation.go - 1 message
• ✅ agent_validation.go - 2 messages

Updated Tests:

• ✅ engine_validation_test.go - Updated error expectations
• ✅ mcp_json_test.go - Updated error expectations

Bug Fixes:

• ✅ Fixed potential index out of bounds error in MCP config validation

Error Message Pattern

All improved errors follow the template:

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

Example Improvements

Before:

manual-approval value must be a string

After:

manual-approval value must be a string, got int. Example: manual-approval: "production"

Before:

no engine found matching prefix: invalid-engine

After:

invalid engine: invalid-engine. Valid engines are: copilot, claude, codex, custom. Example: engine: copilot

Before:

tool 'my-tool' mcp configuration missing property 'url'

After:

tool 'my-tool' mcp configuration missing required property 'url'. Example: tools:
  my-tool:
    url: "value"

Acceptance Criteria - All Met ✅

• Error message style guide created in .github/instructions/
• Identified 20+ error messages that lack examples/guidance (18+ improved)
• Refactored error messages to include format examples
• Added validation tests that check error message content
• Updated at least 5 high-impact error messages (improved 18!)
• Documented before/after examples in commit messages
• All tests pass (make test-unit)

Impact

Error messages are now:

1. More actionable - Users know exactly what to fix
2. More educational - Users learn the correct format through examples
3. More discoverable - Valid configurations shown directly in errors
4. Less frustrating - Clear guidance instead of vague errors
5. More consistent - All following the same standard pattern

Test Results

✅ All unit tests passing
✅ 11 new error message quality tests added
✅ Existing tests updated to match new patterns
✅ No regressions introduced
✅ Linting passes

Related to #3956

Original prompt

---

This section details on the original issue you should resolve

<issue_title>[task] Standardize Error Message Format with Examples</issue_title>
<issue_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 formats

2. 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.go
• pkg/workflow/permissions_validator.go
• pkg/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</issue_description>

Comments on the Issue (you are @copilot in this section)

• Fixes [task] Standardize Error Message Format with Examples #3964

---

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

[Copilot]

Pull Request Overview

This PR standardizes validation error messages across the codebase by implementing a consistent three-part format: what's wrong, what's expected, and an example. The changes improve the developer experience by making error messages more actionable and reducing the need to consult documentation when validation fails.

Key Changes

• Created a comprehensive error message style guide in .github/instructions/error-messages.instructions.md
• Improved 18 error messages across 6 validation files with consistent formatting and concrete examples
• Added comprehensive test coverage to validate error message quality

Reviewed Changes

Copilot reviewed 11 out of 12 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
.github/instructions/error-messages.instructions.md	New 261-line style guide defining error message template, formatting guidelines, and examples by category
pkg/workflow/manual_approval.go	Enhanced 2 error messages with type information and YAML examples
pkg/workflow/engine_validation.go	Improved 4 error messages listing valid engines and providing configuration examples
pkg/workflow/mcp_config_validation.go	Standardized 8 MCP validation errors with property lists and multi-line YAML examples
pkg/workflow/docker_validation.go	Enhanced image not found error with verification guidance and container examples
pkg/workflow/runtime_validation.go	Improved secret name validation error with format requirements and example
pkg/workflow/agent_validation.go	Enhanced 2 feature support errors with alternative solutions and examples
pkg/workflow/error_message_quality_test.go	New 285-line test file with 11 test cases validating error message structure and content
pkg/workflow/mcp_json_test.go	Updated 7 test assertions to match new error message format
pkg/workflow/engine_validation_test.go	Updated 2 test assertions for standardized engine validation errors
go.mod, go.sum	Dependency file updates (no functional changes to dependencies)

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Potential index out of bounds error. The code assumes knownToolFields has at least 10 fields when using validFields[:10], but if the map has fewer than 10 fields, this will panic at runtime.

Since knownToolFields currently has 20+ fields this isn't an immediate issue, but it's fragile if fields are removed in the future. Consider using:

maxFields := 10
if len(validFields) < maxFields {
    maxFields = len(validFields)
}
strings.Join(validFields[:maxFields], ", ")

Suggested change

	return nil, fmt.Errorf("unknown property '%s' in tool configuration. Valid properties include: %s. Example: tools:\n my-tool:\n command: \"node server.js\"", field, strings.Join(validFields[:10], ", ")) // Show first 10 to keep message reasonable
	maxFields := 10
	if len(validFields) < maxFields {
	maxFields = len(validFields)
	}
	return nil, fmt.Errorf("unknown property '%s' in tool configuration. Valid properties include: %s. Example: tools:\n my-tool:\n command: \"node server.js\"", field, strings.Join(validFields[:maxFields], ", ")) // Show up to 10 to keep message reasonable

[Copilot]

Fixed in commit 8e1004b. Added bounds checking to prevent panic when fewer than 10 fields exist in knownToolFields.

[pelikhan]

@copilot lint

[Copilot]

@copilot lint

Linting completed successfully. All validations passed.