diff --git a/pkg/workflow/agent_validation.go b/pkg/workflow/agent_validation.go index 277425bc4a3..058f38ca46b 100644 --- a/pkg/workflow/agent_validation.go +++ b/pkg/workflow/agent_validation.go @@ -90,7 +90,7 @@ func (c *Compiler) validateAgentFile(workflowData *WorkflowData, markdownPath st Column: 1, }, Type: "error", - Message: fmt.Sprintf("agent file '%s' does not exist. Ensure the file exists in the repository and is properly imported.", agentPath), + Message: fmt.Sprintf("🔍 Can't find the agent file '%s'.\n\nPlease check:\n • The file path is correct\n • The file exists in your repository\n • The path is relative to repository root\n\nExample:\n agent:\n file: \".github/agents/developer.md\"", agentPath), }) return errors.New(formattedErr) } @@ -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. 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) + return fmt.Errorf("⚠️ The '%s' tool uses HTTP transport, which isn't supported by the '%s' engine.\n\nWhy? The %s engine only supports stdio (local process) communication.\n\nYour options:\n 1. Switch to an engine that supports HTTP (recommended):\n engine: copilot\n\n 2. Change the tool to use stdio transport:\n tools:\n %s:\n type: stdio\n command: \"node server.js\"\n\nLearn more: https://githubnext.github.io/gh-aw/reference/engines/", toolName, engine.GetID(), engine.GetID(), toolName) } } } @@ -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. Use engine: copilot or remove max-turns from your configuration. Example:\nengine:\n id: copilot\n max-turns: 5", engine.GetID()) + return fmt.Errorf("⚠️ The max-turns feature isn't supported by the '%s' engine.\n\nWhy? This feature requires specific engine capabilities for conversation management.\n\nYour options:\n 1. Switch to Copilot (recommended):\n engine:\n id: copilot\n max-turns: 5\n\n 2. Remove max-turns from your configuration\n\nLearn more: https://githubnext.github.io/gh-aw/reference/engines/#max-turns", engine.GetID()) } // Engine supports max-turns - additional validation could be added here if needed @@ -170,7 +170,7 @@ func (c *Compiler) validateWebSearchSupport(tools map[string]any, engine CodingA // web-search is specified, check if the engine supports it if !engine.SupportsWebSearch() { - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Engine '%s' does not support the web-search tool. See https://githubnext.github.io/gh-aw/guides/web-search/ for alternatives.", engine.GetID()))) + fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("💡 The '%s' engine doesn't support the web-search tool yet.\n\nThe workflow will compile, but web-search won't be available at runtime.\n\nSee alternatives: https://githubnext.github.io/gh-aw/guides/web-search/", engine.GetID()))) c.IncrementWarningCount() } } @@ -230,17 +230,21 @@ func (c *Compiler) validateWorkflowRunBranches(workflowData *WorkflowData, markd } // workflow_run without branches - this is a warning or error depending on mode - message := "workflow_run trigger should include branch restrictions for security and performance.\n\n" + - "Without branch restrictions, the workflow will run for workflow runs on ALL branches,\n" + - "which can cause unexpected behavior and security issues.\n\n" + - "Suggested fix: Add branch restrictions to your workflow_run trigger:\n" + + message := "⚠️ Your workflow_run trigger is missing branch restrictions.\n\n" + + "Why this matters: Without branch restrictions, the workflow will run for ALL branches,\n" + + "which can cause:\n" + + " • Unexpected behavior on feature branches\n" + + " • Wasted CI resources\n" + + " • Potential security issues\n\n" + + "Recommended fix - Add branch restrictions:\n" + "on:\n" + " workflow_run:\n" + " workflows: [\"your-workflow\"]\n" + " types: [completed]\n" + " branches:\n" + " - main\n" + - " - develop" + " - develop\n\n" + + "Learn more: https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#workflow_run" if c.strictMode { // In strict mode, this is an error diff --git a/pkg/workflow/compiler_expression_size_test.go b/pkg/workflow/compiler_expression_size_test.go index 3e12e4755a9..741ca4887d5 100644 --- a/pkg/workflow/compiler_expression_size_test.go +++ b/pkg/workflow/compiler_expression_size_test.go @@ -99,8 +99,8 @@ safe-outputs: // This should fail with an expression size validation error if err == nil { t.Error("Expected error for workflow with oversized expressions, got nil") - } else if !strings.Contains(err.Error(), "exceeds maximum allowed") { - t.Errorf("Expected 'exceeds maximum allowed' error, got: %v", err) + } else if !strings.Contains(err.Error(), "is too large") { + t.Errorf("Expected 'is too large' error, got: %v", err) } else if !strings.Contains(err.Error(), "expression size validation failed") { t.Errorf("Expected 'expression size validation failed' error, got: %v", err) } @@ -146,8 +146,8 @@ safe-outputs: // This should fail with an expression size validation error even without explicit validation enablement if err == nil { t.Error("Expected error for workflow with oversized expressions with default validation settings, got nil") - } else if !strings.Contains(err.Error(), "exceeds maximum allowed") { - t.Errorf("Expected 'exceeds maximum allowed' error with default validation, got: %v", err) + } else if !strings.Contains(err.Error(), "is too large") { + t.Errorf("Expected 'is too large' error with default validation, got: %v", err) } else if !strings.Contains(err.Error(), "expression size validation failed") { t.Errorf("Expected 'expression size validation failed' error with default validation, got: %v", err) } @@ -166,12 +166,12 @@ safe-outputs: actualSize := console.FormatFileSize(testLineSize) maxSizeFormatted := console.FormatFileSize(int64(MaxExpressionSize)) - expectedMessage := fmt.Sprintf("expression value for 'WORKFLOW_MARKDOWN' (%s) exceeds maximum allowed size (%s)", + expectedMessage := fmt.Sprintf("expression value for 'WORKFLOW_MARKDOWN' (%s) is too large size (%s)", actualSize, maxSizeFormatted) // Verify the message contains expected elements - if !strings.Contains(expectedMessage, "exceeds maximum allowed size") { - t.Error("Error message should contain 'exceeds maximum allowed size'") + if !strings.Contains(expectedMessage, "is too large size") { + t.Error("Error message should contain 'is too large size'") } if !strings.Contains(expectedMessage, "KB") { t.Error("Error message should contain size in KB") diff --git a/pkg/workflow/dangerous_permissions_validation.go b/pkg/workflow/dangerous_permissions_validation.go index e37d0670146..853ec3a6cf2 100644 --- a/pkg/workflow/dangerous_permissions_validation.go +++ b/pkg/workflow/dangerous_permissions_validation.go @@ -73,21 +73,33 @@ func findWritePermissions(permissions *Permissions) []PermissionScope { return writePerms } -// formatDangerousPermissionsError formats an error message for write permissions violations +// formatDangerousPermissionsError formats an empathetic error message for write permissions violations func formatDangerousPermissionsError(writePermissions []PermissionScope) error { var lines []string - lines = append(lines, "Write permissions are not allowed.") + lines = append(lines, "🔒 Write permissions detected in your workflow.") + lines = append(lines, "") + lines = append(lines, "Why this matters: For security, workflows use read-only permissions by default.") + lines = append(lines, "Write permissions can modify repository contents and settings, which requires") + lines = append(lines, "explicit opt-in through a feature flag.") lines = append(lines, "") lines = append(lines, "Found write permissions:") for _, scope := range writePermissions { - lines = append(lines, fmt.Sprintf(" - %s: write", scope)) + lines = append(lines, fmt.Sprintf(" • %s: write", scope)) } lines = append(lines, "") - lines = append(lines, "To fix this issue, change write permissions to read:") - lines = append(lines, "permissions:") + lines = append(lines, "You have two options:") + lines = append(lines, "") + lines = append(lines, "1. Change to read-only (recommended):") + lines = append(lines, " permissions:") for _, scope := range writePermissions { - lines = append(lines, fmt.Sprintf(" %s: read", scope)) + lines = append(lines, fmt.Sprintf(" %s: read", scope)) } + lines = append(lines, "") + lines = append(lines, "2. Enable write permissions feature flag:") + lines = append(lines, " features:") + lines = append(lines, " dangerous-permissions-write: true") + lines = append(lines, "") + lines = append(lines, "Learn more: https://githubnext.github.io/gh-aw/reference/permissions/") return fmt.Errorf("%s", strings.Join(lines, "\n")) } diff --git a/pkg/workflow/dangerous_permissions_validation_test.go b/pkg/workflow/dangerous_permissions_validation_test.go index a2652a43f5a..5da94a1b151 100644 --- a/pkg/workflow/dangerous_permissions_validation_test.go +++ b/pkg/workflow/dangerous_permissions_validation_test.go @@ -27,13 +27,13 @@ func TestValidateDangerousPermissions(t *testing.T) { name: "write permission without feature flag - should error", permissions: "permissions:\n contents: write", shouldError: true, - errorContains: "Write permissions are not allowed", + errorContains: "Write permissions detected", }, { name: "multiple write permissions without feature flag - should error", permissions: "permissions:\n contents: write\n issues: write", shouldError: true, - errorContains: "Write permissions are not allowed", + errorContains: "Write permissions detected", }, { name: "write permission with feature flag enabled - should pass", @@ -50,7 +50,7 @@ func TestValidateDangerousPermissions(t *testing.T) { "dangerous-permissions-write": false, }, shouldError: true, - errorContains: "Write permissions are not allowed", + errorContains: "Write permissions detected", }, { name: "shorthand read-all - should pass", @@ -61,7 +61,7 @@ func TestValidateDangerousPermissions(t *testing.T) { name: "shorthand write-all without feature flag - should error", permissions: "permissions: write-all", shouldError: true, - errorContains: "Write permissions are not allowed", + errorContains: "Write permissions detected", }, { name: "shorthand write-all with feature flag - should pass", @@ -198,13 +198,13 @@ func TestFormatDangerousPermissionsError(t *testing.T) { PermissionContents, }, expectedContains: []string{ - "Write permissions are not allowed", + "Write permissions detected", "contents: write", "contents: read", + "dangerous-permissions-write: true", }, expectedNotContain: []string{ - "dangerous-permissions-write: true", - "Option 2", + // Empty - we now include the feature flag option }, }, { @@ -214,15 +214,15 @@ func TestFormatDangerousPermissionsError(t *testing.T) { PermissionIssues, }, expectedContains: []string{ - "Write permissions are not allowed", + "Write permissions detected", "contents: write", "issues: write", "contents: read", "issues: read", + "dangerous-permissions-write: true", }, expectedNotContain: []string{ - "dangerous-permissions-write: true", - "Option 2", + // Empty - we now include the feature flag option }, }, } diff --git a/pkg/workflow/data/action_pins.json b/pkg/workflow/data/action_pins.json index 0262bcec27b..b8a317833e6 100644 --- a/pkg/workflow/data/action_pins.json +++ b/pkg/workflow/data/action_pins.json @@ -1,3 +1,194 @@ { - "entries": {} + "entries": { + "actions/ai-inference@v2": { + "repo": "actions/ai-inference", + "version": "v2", + "sha": "334892bb203895caaed82ec52d23c1ed9385151e" + }, + "actions/attest-build-provenance@v2": { + "repo": "actions/attest-build-provenance", + "version": "v2", + "sha": "96b4a1ef7235a096b17240c259729fdd70c83d45" + }, + "actions/cache/restore@v4.3.0": { + "repo": "actions/cache/restore", + "version": "v4.3.0", + "sha": "0057852bfaa89a56745cba8c7296529d2fc39830" + }, + "actions/cache/save@v4.3.0": { + "repo": "actions/cache/save", + "version": "v4.3.0", + "sha": "0057852bfaa89a56745cba8c7296529d2fc39830" + }, + "actions/cache@v4.3.0": { + "repo": "actions/cache", + "version": "v4.3.0", + "sha": "0057852bfaa89a56745cba8c7296529d2fc39830" + }, + "actions/checkout@v4": { + "repo": "actions/checkout", + "version": "v4", + "sha": "34e114876b0b11c390a56381ad16ebd13914f8d5" + }, + "actions/checkout@v5.0.1": { + "repo": "actions/checkout", + "version": "v5.0.1", + "sha": "93cb6efe18208431cddfb8368fd83d5badbf9bfd" + }, + "actions/create-github-app-token@v2.2.1": { + "repo": "actions/create-github-app-token", + "version": "v2.2.1", + "sha": "29824e69f54612133e76f7eaac726eef6c875baf" + }, + "actions/download-artifact@v6.0.0": { + "repo": "actions/download-artifact", + "version": "v6.0.0", + "sha": "018cc2cf5baa6db3ef3c5f8a56943fffe632ef53" + }, + "actions/github-script@v7.0.1": { + "repo": "actions/github-script", + "version": "v7.1.0", + "sha": "f28e40c7f34bde8b3046d885e986cb6290c5673b" + }, + "actions/github-script@v8.0.0": { + "repo": "actions/github-script", + "version": "v8.0.0", + "sha": "ed597411d8f924073f98dfc5c65a23a2325f34cd" + }, + "actions/setup-dotnet@v4": { + "repo": "actions/setup-dotnet", + "version": "v4.3.1", + "sha": "67a3573c9a986a3f9c594539f4ab511d57bb3ce9" + }, + "actions/setup-go@v6": { + "repo": "actions/setup-go", + "version": "v6", + "sha": "7a3fe6cf4cb3a834922a1244abfce67bcef6a0c5" + }, + "actions/setup-go@v6.1.0": { + "repo": "actions/setup-go", + "version": "v6.1.0", + "sha": "4dc6199c7b1a012772edbd06daecab0f50c9053c" + }, + "actions/setup-java@v4": { + "repo": "actions/setup-java", + "version": "v4.8.0", + "sha": "c1e323688fd81a25caa38c78aa6df2d33d3e20d9" + }, + "actions/setup-node@v6": { + "repo": "actions/setup-node", + "version": "v6", + "sha": "6044e13b5dc448c55e2357c09f80417699197238" + }, + "actions/setup-node@v6.1.0": { + "repo": "actions/setup-node", + "version": "v6.1.0", + "sha": "395ad3262231945c25e8478fd5baf05154b1d79f" + }, + "actions/setup-python@v5.6.0": { + "repo": "actions/setup-python", + "version": "v5.6.0", + "sha": "a26af69be951a213d495a4c3e4e4022e16d87065" + }, + "actions/upload-artifact@v4": { + "repo": "actions/upload-artifact", + "version": "v4.6.2", + "sha": "ea165f8d65b6e75b540449e92b4886f43607fa02" + }, + "actions/upload-artifact@v5.0.0": { + "repo": "actions/upload-artifact", + "version": "v5.0.0", + "sha": "330a01c490aca151604b8cf639adc76d48f6c5d4" + }, + "actions/upload-artifact@v6.0.0": { + "repo": "actions/upload-artifact", + "version": "v6.0.0", + "sha": "b7c566a772e6b6bfb58ed0dc250532a479d7789f" + }, + "anchore/sbom-action@v0.20.10": { + "repo": "anchore/sbom-action", + "version": "v0.20.10", + "sha": "fbfd9c6c189226748411491745178e0c2017392d" + }, + "anchore/sbom-action@v0.20.11": { + "repo": "anchore/sbom-action", + "version": "v0.20.11", + "sha": "43a17d6e7add2b5535efe4dcae9952337c479a93" + }, + "astral-sh/setup-uv@v5.4.2": { + "repo": "astral-sh/setup-uv", + "version": "v5.4.2", + "sha": "d4b2f3b6ecc6e67c4457f6d3e41ec42d3d0fcb86" + }, + "cli/gh-extension-precompile@v2.1.0": { + "repo": "cli/gh-extension-precompile", + "version": "v2.1.0", + "sha": "9e2237c30f869ad3bcaed6a4be2cd43564dd421b" + }, + "denoland/setup-deno@v2": { + "repo": "denoland/setup-deno", + "version": "v2.0.3", + "sha": "e95548e56dfa95d4e1a28d6f422fafe75c4c26fb" + }, + "docker/build-push-action@v6": { + "repo": "docker/build-push-action", + "version": "v6", + "sha": "263435318d21b8e681c14492fe198d362a7d2c83" + }, + "docker/login-action@v3": { + "repo": "docker/login-action", + "version": "v3", + "sha": "5e57cd118135c172c3672efd75eb46360885c0ef" + }, + "docker/setup-buildx-action@v3": { + "repo": "docker/setup-buildx-action", + "version": "v3", + "sha": "8d2750c68a42422c14e847fe6c8ac0403b4cbd6f" + }, + "erlef/setup-beam@v1": { + "repo": "erlef/setup-beam", + "version": "v1.20.4", + "sha": "dff508cca8ce57162e7aa6c4769a4f97c2fed638" + }, + "github/codeql-action/upload-sarif@v3": { + "repo": "github/codeql-action/upload-sarif", + "version": "v3.31.9", + "sha": "70c165ac82ca0e33a10e9741508dd0ccb4dcf080" + }, + "github/stale-repos@v3": { + "repo": "github/stale-repos", + "version": "v3", + "sha": "3477b6488008d9411aaf22a0924ec7c1f6a69980" + }, + "github/stale-repos@v3.0.2": { + "repo": "github/stale-repos", + "version": "v3.0.2", + "sha": "a21e55567b83cf3c3f3f9085d3038dc6cee02598" + }, + "haskell-actions/setup@v2": { + "repo": "haskell-actions/setup", + "version": "v2.9.1", + "sha": "55073cbd0e96181a9abd6ff4e7d289867dffc98d" + }, + "oven-sh/setup-bun@v2": { + "repo": "oven-sh/setup-bun", + "version": "v2.0.2", + "sha": "735343b667d3e6f658f44d0eca948eb6282f2b76" + }, + "ruby/setup-ruby@v1": { + "repo": "ruby/setup-ruby", + "version": "v1.275.0", + "sha": "d354de180d0c9e813cfddfcbdc079945d4be589b" + }, + "super-linter/super-linter@v8.2.1": { + "repo": "super-linter/super-linter", + "version": "v8.2.1", + "sha": "2bdd90ed3262e023ac84bf8fe35dc480721fc1f2" + }, + "super-linter/super-linter@v8.3.1": { + "repo": "super-linter/super-linter", + "version": "v8.3.1", + "sha": "47984f49b4e87383eed97890fe2dca6063bbd9c3" + } + } } diff --git a/pkg/workflow/engine_agent_import_test.go b/pkg/workflow/engine_agent_import_test.go index cb10d95b1e1..4cb30504dd7 100644 --- a/pkg/workflow/engine_agent_import_test.go +++ b/pkg/workflow/engine_agent_import_test.go @@ -252,8 +252,8 @@ This is a test agent file. err := compiler.validateAgentFile(workflowData, workflowPath) if err == nil { t.Error("Expected error for non-existent agent file, got nil") - } else if !strings.Contains(err.Error(), "does not exist") { - t.Errorf("Expected 'does not exist' error, got: %v", err) + } else if !strings.Contains(err.Error(), "Can't find the agent file") { + t.Errorf("Expected 'Can't find the agent file' error, got: %v", err) } }) diff --git a/pkg/workflow/engine_validation.go b/pkg/workflow/engine_validation.go index 164cc60a485..131560c82b2 100644 --- a/pkg/workflow/engine_validation.go +++ b/pkg/workflow/engine_validation.go @@ -64,8 +64,8 @@ func (c *Compiler) validateEngine(engineID string) error { } engineValidationLog.Printf("Engine ID %s not found: %v", engineID, err) - // Provide helpful error with valid options - return fmt.Errorf("invalid engine: %s. Valid engines are: copilot, claude, codex, custom. Example: engine: copilot", engineID) + // Provide empathetic, educational error with valid options + return fmt.Errorf("🤔 Hmm, we don't recognize the engine '%s'.\n\nValid options are:\n • copilot (GitHub Copilot)\n • claude (Anthropic Claude)\n • codex (OpenAI Codex)\n • custom (your own engine)\n\nExample:\n engine: copilot\n\nNeed help choosing? See: https://githubnext.github.io/gh-aw/reference/engines/", engineID) } // validateSingleEngineSpecification validates that only one engine field exists across all files @@ -90,7 +90,7 @@ func (c *Compiler) validateSingleEngineSpecification(mainEngineSetting string, i } if len(allEngines) > 1 { - return "", fmt.Errorf("multiple engine fields found (%d engine specifications detected). Only one engine field is allowed across the main workflow and all included files. Remove duplicate engine specifications to keep only one. Example: engine: copilot", len(allEngines)) + return "", fmt.Errorf("⚠️ Multiple engine specifications detected (%d found).\n\nWhy this matters: Workflows can only use one AI engine at a time to ensure consistent behavior.\n\nPlease remove duplicate engine fields across your main workflow and included files, keeping only one.\n\nExample:\n engine: copilot\n\nTip: Check both your main .md file and any included files for engine: settings", len(allEngines)) } // Exactly one engine found - parse and return it @@ -101,7 +101,7 @@ func (c *Compiler) validateSingleEngineSpecification(mainEngineSetting string, i // Must be from included file var firstEngine any if err := json.Unmarshal([]byte(includedEnginesJSON[0]), &firstEngine); err != nil { - return "", fmt.Errorf("failed to parse included engine configuration: %w. Expected string or object format. Example (string): engine: copilot or (object): engine:\\n id: copilot\\n model: gpt-4", err) + return "", fmt.Errorf("💡 Having trouble parsing the engine configuration from your included file.\n\nEngine configs can be either a simple string or an object format:\n\nString format:\n engine: copilot\n\nObject format:\n engine:\n id: copilot\n model: gpt-4\n\nError details: %w", err) } // Handle string format @@ -116,5 +116,5 @@ func (c *Compiler) validateSingleEngineSpecification(mainEngineSetting string, i } } - return "", fmt.Errorf("invalid engine configuration in included file, missing or invalid 'id' field. Expected string or object with 'id' field. Example (string): engine: copilot or (object): engine:\\n id: copilot\\n model: gpt-4") + return "", fmt.Errorf("💡 The engine configuration in your included file needs an 'id' field.\n\nYou can use either format:\n\nString format (simple):\n engine: copilot\n\nObject format (with options):\n engine:\n id: copilot\n model: gpt-4\n\nThe 'id' field tells us which AI engine to use") } diff --git a/pkg/workflow/engine_validation_test.go b/pkg/workflow/engine_validation_test.go index 027cb507131..3ff935c6e7b 100644 --- a/pkg/workflow/engine_validation_test.go +++ b/pkg/workflow/engine_validation_test.go @@ -42,13 +42,13 @@ func TestValidateEngine(t *testing.T) { name: "invalid engine ID", engineID: "invalid-engine", expectError: true, - errorMsg: "invalid engine", + errorMsg: "don't recognize the engine", }, { name: "unknown engine ID", engineID: "gpt-7", expectError: true, - errorMsg: "invalid engine", + errorMsg: "don't recognize the engine", }, } @@ -137,7 +137,7 @@ func TestValidateSingleEngineSpecification(t *testing.T) { includedEnginesJSON: []string{`"claude"`}, expectedEngine: "", expectError: true, - errorMsg: "multiple engine fields found", + errorMsg: "Multiple engine specifications detected", }, { name: "multiple engines in different included files", @@ -145,7 +145,7 @@ func TestValidateSingleEngineSpecification(t *testing.T) { includedEnginesJSON: []string{`"copilot"`, `"claude"`}, expectedEngine: "", expectError: true, - errorMsg: "multiple engine fields found", + errorMsg: "Multiple engine specifications detected", }, { name: "empty string in main engine setting", @@ -167,7 +167,7 @@ func TestValidateSingleEngineSpecification(t *testing.T) { includedEnginesJSON: []string{`{invalid json}`}, expectedEngine: "", expectError: true, - errorMsg: "failed to parse", + errorMsg: "trouble parsing the engine configuration", }, { name: "included engine with invalid object format (no id)", diff --git a/pkg/workflow/features_validation.go b/pkg/workflow/features_validation.go index 391c3835609..4af6fcf6b37 100644 --- a/pkg/workflow/features_validation.go +++ b/pkg/workflow/features_validation.go @@ -65,7 +65,7 @@ func validateActionTag(value any) error { // Convert to string strVal, ok := value.(string) if !ok { - return fmt.Errorf("action-tag must be a string, got %T", value) + return fmt.Errorf("💡 The action-tag feature needs to be a string (got %T).\n\nExample:\n features:\n action-tag: \"abc123...\" # full 40-char SHA", value) } // Allow empty string (falls back to version) @@ -75,7 +75,7 @@ func validateActionTag(value any) error { // Validate it's a full SHA (40 hex characters) if !isValidFullSHA(strVal) { - return fmt.Errorf("action-tag must be a full 40-character commit SHA, got %q (length: %d). Short SHAs are not allowed. Use 'git rev-parse ' to get the full SHA", strVal, len(strVal)) + return fmt.Errorf("🔒 The action-tag must be a full 40-character commit SHA (got %q with length %d).\n\nWhy? Short SHAs can be ambiguous and pose security risks.\n\nTo get the full SHA:\n git rev-parse \n\nExample:\n features:\n action-tag: \"1234567890abcdef1234567890abcdef12345678\"\n\nLearn more: https://githubnext.github.io/gh-aw/reference/features/#action-tag", strVal, len(strVal)) } return nil diff --git a/pkg/workflow/features_validation_test.go b/pkg/workflow/features_validation_test.go index 7429eb91aed..b1763322517 100644 --- a/pkg/workflow/features_validation_test.go +++ b/pkg/workflow/features_validation_test.go @@ -94,37 +94,37 @@ func TestValidateActionTag(t *testing.T) { name: "invalid - short SHA (7 chars)", value: "5c3428a", expectError: true, - errorMsg: "action-tag must be a full 40-character commit SHA", + errorMsg: "must be a full 40-character commit SHA", }, { name: "invalid - short SHA (8 chars)", value: "abc123de", expectError: true, - errorMsg: "action-tag must be a full 40-character commit SHA", + errorMsg: "must be a full 40-character commit SHA", }, { name: "invalid - version tag instead of SHA", value: "v1.0.0", expectError: true, - errorMsg: "action-tag must be a full 40-character commit SHA", + errorMsg: "must be a full 40-character commit SHA", }, { name: "invalid - not a string", value: 12345, expectError: true, - errorMsg: "action-tag must be a string", + errorMsg: "needs to be a string", }, { name: "invalid - boolean", value: true, expectError: true, - errorMsg: "action-tag must be a string", + errorMsg: "needs to be a string", }, { name: "invalid - uppercase SHA", value: "ABCDEF0123456789ABCDEF0123456789ABCDEF01", expectError: true, - errorMsg: "action-tag must be a full 40-character commit SHA", + errorMsg: "must be a full 40-character commit SHA", }, } @@ -180,7 +180,7 @@ func TestValidateFeatures(t *testing.T) { }, }, expectError: true, - errorMsg: "action-tag must be a full 40-character commit SHA", + errorMsg: "must be a full 40-character commit SHA", }, { name: "invalid action-tag - version tag", @@ -190,7 +190,7 @@ func TestValidateFeatures(t *testing.T) { }, }, expectError: true, - errorMsg: "action-tag must be a full 40-character commit SHA", + errorMsg: "must be a full 40-character commit SHA", }, { name: "empty action-tag is allowed", diff --git a/pkg/workflow/firewall_test.go b/pkg/workflow/firewall_test.go index 6771b25f1cb..8d81c649384 100644 --- a/pkg/workflow/firewall_test.go +++ b/pkg/workflow/firewall_test.go @@ -42,43 +42,43 @@ func TestValidateLogLevel(t *testing.T) { name: "invalid log-level: verbose", logLevel: "verbose", expectErr: true, - errMsg: "invalid log-level 'verbose'", + errMsg: "isn't recognized", }, { name: "invalid log-level: trace", logLevel: "trace", expectErr: true, - errMsg: "invalid log-level 'trace'", + errMsg: "isn't recognized", }, { name: "invalid log-level: warning", logLevel: "warning", expectErr: true, - errMsg: "invalid log-level 'warning'", + errMsg: "isn't recognized", }, { name: "invalid log-level: fatal", logLevel: "fatal", expectErr: true, - errMsg: "invalid log-level 'fatal'", + errMsg: "isn't recognized", }, { name: "invalid log-level: DEBUG (uppercase)", logLevel: "DEBUG", expectErr: true, - errMsg: "invalid log-level 'DEBUG'", + errMsg: "isn't recognized", }, { name: "invalid log-level: Info (mixed case)", logLevel: "Info", expectErr: true, - errMsg: "invalid log-level 'Info'", + errMsg: "isn't recognized", }, { name: "invalid log-level: random string", logLevel: "random", expectErr: true, - errMsg: "invalid log-level 'random'", + errMsg: "isn't recognized", }, } @@ -198,7 +198,7 @@ func TestValidateFirewallConfig(t *testing.T) { }, }, expectErr: true, - errMsg: "invalid log-level 'verbose'", + errMsg: "isn't recognized", }, { name: "invalid log-level: trace", @@ -211,7 +211,7 @@ func TestValidateFirewallConfig(t *testing.T) { }, }, expectErr: true, - errMsg: "invalid log-level 'trace'", + errMsg: "isn't recognized", }, { name: "invalid log-level: DEBUG (uppercase)", @@ -224,7 +224,7 @@ func TestValidateFirewallConfig(t *testing.T) { }, }, expectErr: true, - errMsg: "invalid log-level 'DEBUG'", + errMsg: "isn't recognized", }, } @@ -260,12 +260,12 @@ func TestValidateLogLevelErrorMessageQuality(t *testing.T) { // Check that error message contains key information requiredStrings := []string{ - "verbose", // The invalid value - "invalid log-level", // Clear error type - "debug", // Valid option 1 - "info", // Valid option 2 - "warn", // Valid option 3 - "error", // Valid option 4 + "verbose", // The invalid value + "isn't", // Empathetic phrasing + "debug", // Valid option 1 + "info", // Valid option 2 + "warn", // Valid option 3 + "error", // Valid option 4 } for _, required := range requiredStrings { @@ -274,8 +274,8 @@ func TestValidateLogLevelErrorMessageQuality(t *testing.T) { } } - // Check that error message is concise and helpful - if len(errMsg) > 200 { + // Check that error message is concise and helpful (but empathetic messages are longer) + if len(errMsg) > 400 { t.Errorf("Error message is too long (%d chars): %q", len(errMsg), errMsg) } } diff --git a/pkg/workflow/firewall_validation.go b/pkg/workflow/firewall_validation.go index c912306f6b1..89d01e3de61 100644 --- a/pkg/workflow/firewall_validation.go +++ b/pkg/workflow/firewall_validation.go @@ -26,5 +26,5 @@ func ValidateLogLevel(level string) error { return nil } } - return fmt.Errorf("invalid log-level '%s', must be one of: %v", level, valid) + return fmt.Errorf("💡 The log-level '%s' isn't recognized.\n\nValid log levels:\n • debug - Most detailed, useful for troubleshooting\n • info - Standard logging (default)\n • warn - Warnings only\n • error - Errors only\n\nExample:\n network:\n firewall:\n log-level: info", level) } diff --git a/pkg/workflow/mcp_config_validation.go b/pkg/workflow/mcp_config_validation.go index ed37705c598..028c6e3ccd1 100644 --- a/pkg/workflow/mcp_config_validation.go +++ b/pkg/workflow/mcp_config_validation.go @@ -168,7 +168,7 @@ func getRawMCPConfig(toolConfig map[string]any) (map[string]any, error) { if len(validFields) < maxFields { maxFields = len(validFields) } - return nil, fmt.Errorf("unknown property '%s' in tool configuration. Valid properties include: %s. Example:\nmcp-servers:\n my-tool:\n command: \"node server.js\"\n args: [\"--verbose\"]", field, strings.Join(validFields[:maxFields], ", ")) // Show up to 10 to keep message reasonable + return nil, fmt.Errorf("🤔 Unknown property '%s' in your tool configuration.\n\nValid properties include: %s\n\nExample:\n tools:\n my-tool:\n command: \"node server.js\"\n args: [\"--verbose\"]", field, strings.Join(validFields[:maxFields], ", ")) // Show up to 10 to keep message reasonable } } @@ -203,10 +203,10 @@ func getTypeString(value any) string { // validateStringProperty validates that a property is a string and returns appropriate error message func validateStringProperty(toolName, propertyName string, value any, exists bool) error { if !exists { - return fmt.Errorf("tool '%s' mcp configuration missing required property '%s'. Example:\nmcp-servers:\n %s:\n %s: \"value\"", toolName, propertyName, toolName, propertyName) + return fmt.Errorf("💡 The MCP server '%s' is missing the '%s' property.\n\nExample:\n tools:\n %s:\n %s: \"value\"", toolName, propertyName, toolName, propertyName) } if _, ok := value.(string); !ok { - return fmt.Errorf("tool '%s' mcp configuration property '%s' must be a string, got %T. Example:\nmcp-servers:\n %s:\n %s: \"my-value\"", toolName, propertyName, value, toolName, propertyName) + return fmt.Errorf("💡 The '%s' property for MCP server '%s' needs to be a string (got %T).\n\nExample:\n tools:\n %s:\n %s: \"my-value\"", propertyName, toolName, value, toolName, propertyName) } return nil } @@ -220,7 +220,7 @@ func validateMCPRequirements(toolName string, mcpConfig map[string]any, toolConf if hasType { // Explicit type provided - validate it's a string if _, ok := mcpType.(string); !ok { - return fmt.Errorf("tool '%s' mcp configuration 'type' must be a string, got %T. Valid types per MCP Gateway Specification: stdio, http. Note: 'local' is accepted for backward compatibility and treated as 'stdio'. Example:\nmcp-servers:\n %s:\n type: \"stdio\"\n command: \"node server.js\"", toolName, mcpType, toolName) + return fmt.Errorf("💡 The 'type' field for MCP server '%s' needs to be a string (got %T).\n\nValid types per MCP Gateway Specification:\n • stdio - Local process communication\n • http - HTTP-based communication\n\nNote: 'local' is accepted for backward compatibility and treated as 'stdio'.\n\nExample:\n tools:\n %s:\n type: \"stdio\"\n command: \"node server.js\"", toolName, mcpType, toolName) } typeStr = mcpType.(string) } else { @@ -232,7 +232,7 @@ func validateMCPRequirements(toolName string, mcpConfig map[string]any, toolConf } else if _, hasContainer := mcpConfig["container"]; hasContainer { typeStr = "stdio" } else { - return fmt.Errorf("tool '%s' unable to determine MCP type: missing type, url, command, or container. Example:\nmcp-servers:\n %s:\n command: \"node server.js\"\n args: [\"--port\", \"3000\"]", toolName, toolName) + return fmt.Errorf("💡 Unable to determine the MCP type for '%s'.\n\nMCP servers need one of these:\n • type: \"stdio\" or \"http\"\n • url: \"https://...\" (implies http)\n • command: \"node ...\" (implies stdio)\n • container: \"registry/image\" (implies stdio)\n\nExample:\n tools:\n %s:\n command: \"node server.js\"\n args: [\"--port\", \"3000\"]\n\nLearn more: https://githubnext.github.io/gh-aw/guides/mcp-servers/", toolName, toolName) } } @@ -243,7 +243,7 @@ func validateMCPRequirements(toolName string, mcpConfig map[string]any, toolConf // Validate type is one of the supported types if !parser.IsMCPType(typeStr) { - return fmt.Errorf("tool '%s' mcp configuration 'type' must be one of: stdio, http (per MCP Gateway Specification). Note: 'local' is accepted for backward compatibility and treated as 'stdio'. Got: %s. Example:\nmcp-servers:\n %s:\n type: \"stdio\"\n command: \"node server.js\"", toolName, typeStr, toolName) + return fmt.Errorf("💡 The MCP type '%s' for server '%s' isn't recognized.\n\nValid types per MCP Gateway Specification:\n • stdio - Local process communication\n • http - HTTP-based communication\n\nNote: 'local' is accepted for backward compatibility and treated as 'stdio'.\n\nExample:\n tools:\n %s:\n type: \"stdio\"\n command: \"node server.js\"\n\nLearn more: https://githubnext.github.io/gh-aw/guides/mcp-servers/", typeStr, toolName, toolName) } // Validate type-specific requirements @@ -254,7 +254,7 @@ func validateMCPRequirements(toolName string, mcpConfig map[string]any, toolConf // HTTP type cannot use container field if _, hasContainer := mcpConfig["container"]; hasContainer { - return fmt.Errorf("tool '%s' mcp configuration with type 'http' cannot use 'container' field. HTTP MCP uses URL endpoints, not containers. Example:\nmcp-servers:\n %s:\n type: http\n url: \"https://api.example.com/mcp\"\n headers:\n Authorization: \"Bearer ${{ secrets.API_KEY }}\"", toolName, toolName) + return fmt.Errorf("💡 HTTP MCP servers like '%s' can't use the 'container' field.\n\nWhy? HTTP MCP uses URL endpoints, not containers.\n\nExample:\n tools:\n %s:\n type: http\n url: \"https://api.example.com/mcp\"\n headers:\n Authorization: \"Bearer ${{ secrets.API_KEY }}\"", toolName, toolName) } return validateStringProperty(toolName, "url", url, hasURL) @@ -265,7 +265,7 @@ func validateMCPRequirements(toolName string, mcpConfig map[string]any, toolConf container, hasContainer := mcpConfig["container"] if hasCommand && hasContainer { - return fmt.Errorf("tool '%s' mcp configuration cannot specify both 'container' and 'command'. Choose one. Example:\nmcp-servers:\n %s:\n command: \"node server.js\"\nOr use container:\nmcp-servers:\n %s:\n container: \"my-registry/my-tool\"\n version: \"latest\"", toolName, toolName, toolName) + return fmt.Errorf("💡 MCP server '%s' has both 'command' and 'container' specified.\n\nWhy? We need to know exactly one way to start your MCP server.\n\nChoose one approach:\n\nOption 1 - Use command:\n tools:\n %s:\n command: \"node server.js\"\n\nOption 2 - Use container:\n tools:\n %s:\n container: \"my-registry/my-tool\"\n version: \"latest\"", toolName, toolName, toolName) } if hasCommand { @@ -277,7 +277,7 @@ func validateMCPRequirements(toolName string, mcpConfig map[string]any, toolConf return err } } else { - return fmt.Errorf("tool '%s' mcp configuration must specify either 'command' or 'container'. Example:\nmcp-servers:\n %s:\n command: \"node server.js\"\n args: [\"--port\", \"3000\"]\nOr use container:\nmcp-servers:\n %s:\n container: \"my-registry/my-tool\"\n version: \"latest\"", toolName, toolName, toolName) + return fmt.Errorf("💡 MCP server '%s' needs a way to start.\n\nStdio MCP servers need either 'command' or 'container' specified.\n\nExample with command:\n tools:\n %s:\n command: \"node server.js\"\n args: [\"--port\", \"3000\"]\n\nOr with container:\n tools:\n %s:\n container: \"my-registry/my-tool\"\n version: \"latest\"\n\nLearn more: https://githubnext.github.io/gh-aw/guides/mcp-servers/", toolName, toolName, toolName) } } diff --git a/pkg/workflow/runtime_validation.go b/pkg/workflow/runtime_validation.go index 64a31b37529..e2dff9c6565 100644 --- a/pkg/workflow/runtime_validation.go +++ b/pkg/workflow/runtime_validation.go @@ -79,10 +79,10 @@ func (c *Compiler) validateExpressionSizes(yamlContent string) error { var errorMsg string if key != "" { - errorMsg = fmt.Sprintf("expression value for %q (%s) exceeds maximum allowed size (%s) at line %d. GitHub Actions has a 21KB limit for expression values including environment variables. Consider chunking the content or using artifacts instead.", - key, actualSize, maxSizeFormatted, lineNum+1) + errorMsg = fmt.Sprintf("📝 The expression value for %q is too large (%s).\n\nWhy this matters: GitHub Actions has a 21KB limit for expression values including environment variables. This prevents workflows from failing at runtime.\n\nCurrent size: %s\nMaximum allowed: %s\nFound at line: %d\n\nTo fix, consider:\n • Breaking the content into smaller chunks\n • Using GitHub Actions artifacts for large data\n • Storing data in files instead of environment variables", + key, actualSize, actualSize, maxSizeFormatted, lineNum+1) } else { - errorMsg = fmt.Sprintf("line %d (%s) exceeds maximum allowed expression size (%s). GitHub Actions has a 21KB limit for expression values.", + errorMsg = fmt.Sprintf("📝 Line %d is too large (%s).\n\nGitHub Actions has a 21KB limit for expression values.\n\nMaximum allowed: %s\n\nConsider breaking this into smaller pieces or using artifacts.", lineNum+1, actualSize, maxSizeFormatted) } @@ -261,7 +261,7 @@ func validateNoDuplicateCacheIDs(caches []CacheMemoryEntry) error { seen := make(map[string]bool) for _, cache := range caches { if seen[cache.ID] { - return fmt.Errorf("duplicate cache-memory ID '%s' found. Each cache must have a unique ID", cache.ID) + return fmt.Errorf("💡 Duplicate cache-memory ID '%s' found.\n\nWhy this matters: Each cache needs a unique ID so we can track it separately.\n\nTo fix: Give each cache a unique ID.\n\nExample:\n tools:\n cache-memory:\n - id: user-preferences\n - id: session-data", cache.ID) } seen[cache.ID] = true } @@ -275,7 +275,7 @@ func validateSecretReferences(secrets []string) error { for _, secret := range secrets { if !secretNamePattern.MatchString(secret) { - return fmt.Errorf("invalid secret name: %s. Secret names must start with an uppercase letter and contain only uppercase letters, numbers, and underscores. Example: MY_SECRET_KEY", secret) + return fmt.Errorf("🔒 The secret name '%s' doesn't follow the required format.\n\nWhy? GitHub secret names must be valid environment variable names for security and compatibility.\n\nRules:\n • Start with an uppercase letter\n • Contain only uppercase letters, numbers, and underscores\n\nExample: MY_SECRET_KEY\n\nLearn more: https://docs.github.com/en/actions/security-guides/encrypted-secrets", secret) } } diff --git a/pkg/workflow/sandbox_experimental_warning_test.go b/pkg/workflow/sandbox_experimental_warning_test.go index f82d8445947..e76ee7e9d7d 100644 --- a/pkg/workflow/sandbox_experimental_warning_test.go +++ b/pkg/workflow/sandbox_experimental_warning_test.go @@ -171,7 +171,7 @@ permissions: # Test Workflow `, expectError: true, - errorMessage: "sandbox-runtime feature is experimental and requires the 'sandbox-runtime' feature flag", + errorMessage: "Sandbox-runtime is experimental and needs a feature flag", }, { name: "sandbox-runtime with feature flag succeeds", @@ -208,7 +208,7 @@ permissions: # Test Workflow `, expectError: true, - errorMessage: "sandbox-runtime feature is experimental and requires the 'sandbox-runtime' feature flag", + errorMessage: "Sandbox-runtime is experimental and needs a feature flag", }, { name: "sandbox default does not require feature flag", diff --git a/pkg/workflow/sandbox_mounts_test.go b/pkg/workflow/sandbox_mounts_test.go index fcc1ec43fad..c4650bdda33 100644 --- a/pkg/workflow/sandbox_mounts_test.go +++ b/pkg/workflow/sandbox_mounts_test.go @@ -41,37 +41,37 @@ func TestValidateMountsSyntax(t *testing.T) { name: "invalid mount - too few parts", mounts: []string{"/host/path:/container/path"}, wantErr: true, - errMsg: "invalid mount syntax", + errMsg: "mount syntax", }, { name: "invalid mount - too many parts", mounts: []string{"/host/path:/container/path:ro:extra"}, wantErr: true, - errMsg: "invalid mount syntax", + errMsg: "mount syntax", }, { name: "invalid mount - empty source", mounts: []string{":/container/path:ro"}, wantErr: true, - errMsg: "source path is empty", + errMsg: "missing the source path", }, { name: "invalid mount - empty destination", mounts: []string{"/host/path::ro"}, wantErr: true, - errMsg: "destination path is empty", + errMsg: "missing the destination path", }, { name: "invalid mount - invalid mode", mounts: []string{"/host/path:/container/path:xyz"}, wantErr: true, - errMsg: "mode must be 'ro' (read-only) or 'rw' (read-write)", + errMsg: "Mode must be either", }, { name: "invalid mount - uppercase mode", mounts: []string{"/host/path:/container/path:RO"}, wantErr: true, - errMsg: "mode must be 'ro' (read-only) or 'rw' (read-write)", + errMsg: "Mode must be either", }, { name: "mixed valid and invalid mounts", @@ -80,7 +80,7 @@ func TestValidateMountsSyntax(t *testing.T) { "/invalid:mount", }, wantErr: true, - errMsg: "invalid mount syntax at index 1", + errMsg: "mount syntax at index 1", }, } @@ -186,7 +186,7 @@ func TestSandboxConfigWithMounts(t *testing.T) { }, }, wantErr: true, - errMsg: "invalid mount syntax", + errMsg: "mount syntax", }, { name: "invalid mode in mount", @@ -210,7 +210,7 @@ func TestSandboxConfigWithMounts(t *testing.T) { }, }, wantErr: true, - errMsg: "mode must be 'ro' (read-only) or 'rw' (read-write)", + errMsg: "Mode must be either", }, } diff --git a/pkg/workflow/sandbox_test.go b/pkg/workflow/sandbox_test.go index 99eab5c35a6..5e3e15e809e 100644 --- a/pkg/workflow/sandbox_test.go +++ b/pkg/workflow/sandbox_test.go @@ -252,7 +252,7 @@ func TestValidateSandboxConfig(t *testing.T) { Features: map[string]any{}, }, expectError: true, - errorMsg: "sandbox-runtime feature is experimental", + errorMsg: "Sandbox-runtime is experimental", }, { name: "SRT with feature flag succeeds", @@ -262,6 +262,9 @@ func TestValidateSandboxConfig(t *testing.T) { }, EngineConfig: &EngineConfig{ID: "copilot"}, Features: map[string]any{"sandbox-runtime": true}, + Tools: map[string]any{ + "github": map[string]any{"mode": "remote"}, + }, }, expectError: false, }, @@ -275,7 +278,7 @@ func TestValidateSandboxConfig(t *testing.T) { Features: map[string]any{"sandbox-runtime": true}, }, expectError: true, - errorMsg: "sandbox-runtime is only supported with Copilot engine", + errorMsg: "requires the Copilot engine", }, { name: "SRT with AWF firewall fails", @@ -290,7 +293,7 @@ func TestValidateSandboxConfig(t *testing.T) { }, }, expectError: true, - errorMsg: "sandbox-runtime and AWF firewall cannot be used together", + errorMsg: "Both sandbox-runtime and AWF firewall are enabled", }, } diff --git a/pkg/workflow/sandbox_validation.go b/pkg/workflow/sandbox_validation.go index 3c9e331f5e4..c8ff926d859 100644 --- a/pkg/workflow/sandbox_validation.go +++ b/pkg/workflow/sandbox_validation.go @@ -28,7 +28,7 @@ func validateMountsSyntax(mounts []string) error { // Must have exactly 3 parts: source, destination, mode if len(parts) != 3 { - return fmt.Errorf("invalid mount syntax at index %d: '%s'. Expected format: 'source:destination:mode' (e.g., '/host/path:/container/path:ro')", i, mount) + return fmt.Errorf("💡 The mount syntax at index %d needs adjustment: '%s'\n\nExpected format: 'source:destination:mode'\n\nWhere:\n • source: Path on host machine\n • destination: Path in container\n • mode: Either 'ro' (read-only) or 'rw' (read-write)\n\nExample:\n /host/path:/container/path:ro", i, mount) } source := parts[0] @@ -37,15 +37,15 @@ func validateMountsSyntax(mounts []string) error { // Validate that source and destination are not empty if source == "" { - return fmt.Errorf("invalid mount at index %d: source path is empty in '%s'", i, mount) + return fmt.Errorf("💡 Mount at index %d is missing the source path in '%s'.\n\nThe source path tells us what directory to mount from the host.\n\nFormat: 'source:destination:mode'\nExample: /host/data:/container/data:ro", i, mount) } if dest == "" { - return fmt.Errorf("invalid mount at index %d: destination path is empty in '%s'", i, mount) + return fmt.Errorf("💡 Mount at index %d is missing the destination path in '%s'.\n\nThe destination path tells us where to mount it in the container.\n\nFormat: 'source:destination:mode'\nExample: /host/data:/container/data:ro", i, mount) } // Validate mode is either "ro" or "rw" if mode != "ro" && mode != "rw" { - return fmt.Errorf("invalid mount at index %d: mode must be 'ro' (read-only) or 'rw' (read-write), got '%s' in '%s'", i, mode, mount) + return fmt.Errorf("💡 Mount at index %d has an invalid mode '%s' in '%s'.\n\nMode must be either:\n • 'ro' - read-only (recommended for security)\n • 'rw' - read-write (use with caution)\n\nExample: /host/data:/container/data:ro", i, mode, mount) } sandboxValidationLog.Printf("Validated mount %d: source=%s, dest=%s, mode=%s", i, source, dest, mode) @@ -88,7 +88,7 @@ func validateSandboxConfig(workflowData *WorkflowData) error { if isSRTEnabled(workflowData) { // Check if the sandbox-runtime feature flag is enabled if !isFeatureEnabled(constants.SandboxRuntimeFeatureFlag, workflowData) { - return fmt.Errorf("sandbox-runtime feature is experimental and requires the 'sandbox-runtime' feature flag to be enabled. Set 'features: { sandbox-runtime: true }' in frontmatter or set GH_AW_FEATURES=sandbox-runtime") + return fmt.Errorf("🏗️ Sandbox-runtime is experimental and needs a feature flag.\n\nWhy? This feature is still in active development.\n\nTo enable:\n features:\n sandbox-runtime: true\n\nOr set environment variable:\n GH_AW_FEATURES=sandbox-runtime") } if workflowData.EngineConfig == nil || workflowData.EngineConfig.ID != "copilot" { @@ -96,19 +96,19 @@ func validateSandboxConfig(workflowData *WorkflowData) error { if workflowData.EngineConfig != nil { engineID = workflowData.EngineConfig.ID } - return fmt.Errorf("sandbox-runtime is only supported with Copilot engine (current engine: %s)", engineID) + return fmt.Errorf("⚠️ Sandbox-runtime requires the Copilot engine (currently using: %s).\n\nWhy? This feature is specifically designed for GitHub Copilot.\n\nTo fix:\n engine: copilot\n sandbox: sandbox-runtime", engineID) } // Check for mutual exclusivity with AWF if workflowData.NetworkPermissions != nil && workflowData.NetworkPermissions.Firewall != nil && workflowData.NetworkPermissions.Firewall.Enabled { - return fmt.Errorf("sandbox-runtime and AWF firewall cannot be used together; please use either 'sandbox: sandbox-runtime' or 'network.firewall' but not both") + return fmt.Errorf("⚠️ Both sandbox-runtime and AWF firewall are enabled.\n\nWhy this matters: These two security features can't be used together - choose one approach.\n\nOptions:\n 1. Use sandbox-runtime:\n sandbox: sandbox-runtime\n\n 2. Use AWF firewall:\n network:\n firewall:\n enabled: true\n\nBoth provide network security, but use different approaches") } } // Validate config structure if provided if sandboxConfig.Config != nil { if sandboxConfig.Type != SandboxTypeRuntime { - return fmt.Errorf("custom sandbox config can only be provided when type is 'sandbox-runtime'") + return fmt.Errorf("💡 Custom sandbox configuration detected.\n\nCustom sandbox configs are only supported when type is 'sandbox-runtime'.\n\nExample:\n sandbox:\n type: sandbox-runtime\n config:\n # your custom config here") } } @@ -124,7 +124,7 @@ func validateSandboxConfig(workflowData *WorkflowData) error { sandboxConfig.Type != "" if hasExplicitSandboxConfig && !HasMCPServers(workflowData) { - return fmt.Errorf("agent sandbox is enabled but MCP gateway is not enabled. The agent sandbox requires MCP servers to be configured. Add tools that use MCP (e.g., 'github', 'playwright') or disable the sandbox with 'sandbox: false'") + return fmt.Errorf("🏗️ Agent sandbox is enabled, but the MCP gateway isn't configured.\n\nWhy this matters: The agent sandbox requires MCP servers to work properly.\n\nTo fix, either:\n 1. Add MCP tools (recommended):\n tools:\n github:\n mode: remote\n\n 2. Disable the sandbox:\n sandbox: false\n\nLearn more: https://githubnext.github.io/gh-aw/guides/mcp-servers/") } if hasExplicitSandboxConfig { sandboxValidationLog.Print("Sandbox enabled with MCP gateway - validation passed") diff --git a/pkg/workflow/sandbox_validation_test.go b/pkg/workflow/sandbox_validation_test.go index 87f9fb9b213..ae0cc245375 100644 --- a/pkg/workflow/sandbox_validation_test.go +++ b/pkg/workflow/sandbox_validation_test.go @@ -368,7 +368,7 @@ func TestSandboxMCPGatewayValidation(t *testing.T) { Tools: map[string]any{}, // No tools configured }, expectErr: true, - errContains: "agent sandbox is enabled but MCP gateway is not enabled", + errContains: "Agent sandbox is enabled, but the MCP gateway isn't configured", }, { name: "sandbox enabled with MCP servers - should pass", @@ -488,7 +488,7 @@ func TestSandboxMCPGatewayValidation(t *testing.T) { Tools: map[string]any{}, // No tools configured }, expectErr: true, - errContains: "agent sandbox is enabled but MCP gateway is not enabled", + errContains: "Agent sandbox is enabled, but the MCP gateway isn't configured", }, } diff --git a/pkg/workflow/workflow_run_validation_test.go b/pkg/workflow/workflow_run_validation_test.go index bda218c114f..0339ec22f6f 100644 --- a/pkg/workflow/workflow_run_validation_test.go +++ b/pkg/workflow/workflow_run_validation_test.go @@ -63,7 +63,7 @@ Test workflow content.`, strictMode: true, expectError: true, expectWarning: false, - errorContains: "workflow_run trigger should include branch restrictions", + errorContains: "workflow_run trigger is missing branch restrictions", }, { name: "workflow_run with branches - should pass", diff --git a/skills/error-messages/SKILL.md b/skills/error-messages/SKILL.md index 04388ef9bbb..7701eef8a13 100644 --- a/skills/error-messages/SKILL.md +++ b/skills/error-messages/SKILL.md @@ -11,55 +11,90 @@ This guide establishes the standard format for validation error messages in the ## Error Message Template ``` -[what's wrong]. [what's expected]. [example of correct usage] +[empathetic opening with emoji] [what's wrong and why it matters]. + +[context: why this rule exists] + +[what's expected with specific examples] + +[learning resource link if applicable] ``` -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 +Each error message should answer these questions: +1. **Acknowledge the situation** - Use an emoji and empathetic opening (🤔, 💡, ⚠️, 🔒) +2. **What's wrong?** - Clearly state the validation error +3. **Why does this matter?** - Explain the reason (security, best practices, compatibility) +4. **What's expected?** - Explain the valid format or values +5. **How to fix it?** - Provide concrete examples of correct usage +6. **Where to learn more?** - Link to relevant documentation when helpful ## 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) +These examples follow the empathetic template and provide educational guidance: -### Type Validation with Example +### Engine Validation (Empathetic) ```go -return "", fmt.Errorf("manual-approval value must be a string, got %T. Example: manual-approval: \"production\"", val) +return fmt.Errorf("🤔 Hmm, we don't recognize the engine '%s'.\n\nValid options are:\n • copilot (GitHub Copilot)\n • claude (Anthropic Claude)\n • codex (OpenAI Codex)\n • custom (your own engine)\n\nExample:\n engine: copilot\n\nNeed help choosing? See: https://githubnext.github.io/gh-aw/reference/engines/", engineID) ``` ✅ **Why it's good:** -- Shows actual type received (%T) -- Provides concrete YAML example -- Uses proper YAML syntax with quotes +- Empathetic opening with emoji +- Lists all options with descriptions +- Provides learning resource link +- Conversational, non-blaming tone -### Enum Validation with Options +### Security Validation (Empathetic) ```go -return fmt.Errorf("invalid engine: %s. Valid engines are: copilot, claude, codex, custom. Example: engine: copilot", engineID) +return fmt.Errorf("🔒 Write permissions detected.\n\nFor security, workflows use read-only permissions by default. Write permissions require the 'dangerous-permissions-write' feature flag.\n\nWhy? Write permissions can modify repository contents and settings, which needs explicit opt-in.\n\nFound write permissions:\n%s\n\nTo fix, either:\n 1. Change to read permissions:\n permissions:\n contents: read\n\n 2. Or enable the feature flag:\n features:\n dangerous-permissions-write: true\n\nLearn more: https://githubnext.github.io/gh-aw/reference/permissions/", details) ``` ✅ **Why it's good:** -- Lists all valid options -- Provides simplest example -- Uses consistent formatting +- Lock emoji for security context +- Explains *why* the rule exists +- Provides two fix options +- Links to documentation +- Educational and empowering -### MCP Configuration +### MCP Configuration (Empathetic) ```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) +return fmt.Errorf("💡 The MCP server '%s' needs a way to start.\n\nMCP servers using 'stdio' type need either a 'command' or 'container', but not both.\n\nWhy? The command tells us how to launch your MCP server.\n\nExample with command:\n tools:\n %s:\n command: \"node server.js\"\n args: [\"--port\", \"3000\"]\n\nOr with container:\n tools:\n %s:\n container: \"my-registry/my-tool\"\n version: \"latest\"\n\nLearn more: https://githubnext.github.io/gh-aw/guides/mcp-servers/", toolName, toolName, toolName) ``` ✅ **Why it's good:** -- Explains mutual exclusivity -- Shows realistic tool name -- Formats multi-line YAML example - -## Bad Examples +- Light bulb emoji for helpful suggestion +- Explains the requirement clearly +- Shows both valid options +- Includes why the rule exists +- Links to detailed guide + +## Emoji Guidelines + +Choose emojis that match the context and tone: + +- **🤔** - Confusion or something not recognized (invalid values, unknown options) +- **💡** - Helpful suggestion or tip (configuration needs, how to fix) +- **⚠️** - Warning or caution (potential issues, best practices) +- **🔒** - Security-related (permissions, access control, sensitive data) +- **📝** - Documentation or format issues (syntax, structure) +- **🏗️** - Build or configuration setup (missing dependencies, setup requirements) +- **🔍** - Not found or missing (files, resources, references) + +Use emojis sparingly - one per error message is enough. + +## Conversational Tone Guidelines + +Write error messages as if you're a helpful colleague: + +**DO:** +- Use "we" and "you" to be inclusive +- Acknowledge the situation empathetically +- Explain the "why" behind rules +- Offer choices when applicable +- End with a learning opportunity + +**DON'T:** +- Blame the user ("you did this wrong") +- Use overly technical jargon without explanation +- Be condescending or patronizing +- Use multiple emojis in one message +- Make jokes that minimize the issue These examples lack clarity or actionable guidance: