Improve validation for bash and cache-memory type unions

[Copilot]

Enhanced validation quality for bash and cache-memory fields to match the comprehensive validation already in place for permissions and network fields.

Changes

Bash field validation (pkg/workflow/claude_tools.go)

• Added validateBashToolValue() with explicit type checking (null/boolean/array)
• Validates array elements are strings
• Provides contextual error messages with usage examples

Cache-memory field validation (pkg/workflow/cache.go)

• Added validateCacheMemoryToolValue() with structure validation
• Validates retention-days range (1-90)
• Detects duplicate cache IDs across array entries
• Validates unknown fields with suggestions
• Added parseRetentionDays() helper for numeric type coercion

Integration (pkg/workflow/compiler.go, pkg/workflow/tools_validation.go)

• Added ValidateToolsConfiguration() entry point
• Validation runs after tool merging, before compilation
• Follows layered approach: JSON schema catches type errors, custom validation adds semantic checks

Validation Architecture

JSON Schema (layer 1)          →  Type errors, ranges, required fields
Custom Validation (layer 2)    →  Duplicate IDs, business logic, better UX

Error Message Examples

Duplicate cache IDs:

cache-memory: duplicate cache-memory ID 'duplicate' found. Each cache must have a unique ID

Invalid bash type:

bash: must be null, boolean, or array of command patterns. Got string.
Valid examples:
  bash: true  # Allow all commands
  bash: ["git:*", "npm:*"]  # Allow specific commands

Test Coverage

• 43+ unit tests covering all validation paths
• Integration tests verify compilation workflow
• All existing tests pass

Original prompt

---

This section details on the original issue you should resolve

<issue_title>[task] Improve validation for bash and cache-memory type unions</issue_title>
<issue_description>## Objective
Enhance validation quality for bash and cache-memory fields to match the excellent validation implemented for permissions and network fields.

Context

Current validation inconsistency: permissions and network have comprehensive validation with helpful error messages, while bash and cache-memory only perform basic type checking. This creates inconsistent user experience.

Related to #3511

Current Issues

bash field (claude_tools.go)

• Treats any non-array value as nil (type coercion issue)
• No validation of allowed command patterns
• Silent failures on invalid configurations

cache-memory field (cache.go)

• Basic existence check only
• No validation of object structure when object form used
• No validation of array elements when array form used

Approach

For bash field:

1. Add explicit type validation (null, boolean, array)
2. Validate array elements are valid command patterns
3. Add helpful error messages for common mistakes
4. Follow pattern used in permissions.go:345-380

For cache-memory field:

1. Validate object structure (id, key, retention-days fields)
2. Validate array elements have proper structure
3. Add range validation for retention-days (1-90)
4. Provide suggestions when validation fails

Files to Modify

• Update: pkg/workflow/claude_tools.go
  • Enhance bash field validation
  • Add comprehensive error messages
• Update: pkg/workflow/cache.go
  • Enhance cache-memory field validation
  • Add structure validation for object/array forms

Reference Implementation

Study these excellent validation patterns:

• pkg/workflow/permissions.go:345-380 - Comprehensive scope validation
• pkg/workflow/network.go - Strong ecosystem identifier validation

Acceptance Criteria

• bash field validates type explicitly (null/boolean/array)
• bash field provides helpful error messages for invalid configurations
• cache-memory object form validates required fields (id, key)
• cache-memory array form validates each element structure
• retention-days validated to be 1-90 range
• Error messages match quality of permissions/network validation
• Unit tests added for all new validation logic
• Integration tests verify validation catches common mistakes
  Related to [Schema Consistency] 🔍 Schema Consistency Check - November 9, 2025 #3511

AI generated by Plan Command for discussion #3511</issue_description>

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

• Fixes [task] Improve validation for bash and cache-memory type unions #3517

---

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

[Copilot]

Pull Request Overview

This PR introduces a new validation architecture for tool configurations in the workflow compiler, specifically for bash and cache-memory tools. The changes refactor existing validation logic to be more centralized and consistent, while adding comprehensive test coverage.

Key Changes

• Added centralized ValidateToolsConfiguration function to validate tool configurations during compilation
• Extracted bash validation logic into a dedicated validateBashToolValue function in claude_tools.go
• Refactored cache-memory validation by extracting it into validateCacheMemoryToolValue in cache.go with stricter type checking and field validation
• Added comprehensive unit tests (bash_validation_test.go, cache_memory_validation_test.go) and integration tests (tools_validation_integration_test.go)

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
pkg/workflow/tools_validation.go	New centralized validation entry point that validates bash and cache-memory tool configurations
pkg/workflow/compiler.go	Integrated the new validation call after MCP validation and before HTTP transport validation
pkg/workflow/claude_tools.go	Added validateBashToolValue function with defensive validation in the engine expansion code
pkg/workflow/cache.go	Refactored validateCacheMemoryToolValue with stricter validation, type checking, and field validation
pkg/workflow/bash_validation_test.go	Comprehensive unit tests for bash tool validation covering all value types and error cases
pkg/workflow/cache_memory_validation_test.go	Comprehensive unit tests for cache-memory validation covering all configurations and error scenarios
pkg/workflow/tools_validation_integration_test.go	Integration tests verifying end-to-end compilation with various tool configurations

---

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

[Copilot]

Bug: When an object configuration provides a custom id but no key, the default key generated will incorrectly use "default" as the ID instead of the custom ID.

The issue is that the key is initialized at line 67 with generateDefaultCacheKey("default") before the ID is parsed from the config (lines 71-77). This means that a configuration like:

cache-memory:
  id: session

Would generate key memory-${{ github.workflow }}-${{ github.run_id }} (using "default") instead of memory-session-${{ github.workflow }}-${{ github.run_id }}.

Suggested fix: Parse the id field first, then generate the default key based on that ID. The array configuration handling (lines 148-161) demonstrates the correct approach:

1. Parse the ID
2. Set default if empty
3. Generate key using the final ID

[Copilot]

Code duplication: The validFields map is defined identically in three places (object handling line 113-118, and array handling line 197-202). This duplicated logic makes maintenance harder and error-prone if the valid fields ever need to change.

Consider extracting this to a package-level variable or a helper function:

var cacheMemoryValidFields = map[string]bool{
    "id":             true,
    "key":            true,
    "description":    true,
    "retention-days": true,
}

func validateCacheMemoryFields(fields map[string]any, prefix string) error {
    for field := range fields {
        if !cacheMemoryValidFields[field] {
            return fmt.Errorf("%sunknown field '%s'. Valid fields: id, key, description, retention-days", prefix, field)
        }
    }
    return nil
}