Consolidate duplicate config validation logic into centralized rules package by Copilot · Pull Request #208 · github/gh-aw-mcpg

Copilot · 2026-01-13T01:20:55Z

Config validation had three files (env_validation.go, schema_validation.go, validation.go) with duplicate implementations of port range, timeout, and mount format validation. Each change to validation constraints required updates in 3 places.

Changes

Created internal/config/rules/ package with centralized validation functions:

PortRange(port, jsonPath) - validates 1-65535 range
TimeoutPositive(timeout, fieldName, jsonPath) - validates ≥1
MountFormat(mount, jsonPath, index) - validates source:dest:mode format
Shared ValidationError type with consistent error messaging

Refactored validation files to use centralized rules:

env_validation.go - port validation now calls rules.PortRange()
schema_validation.go - port and timeout validation use rules package
validation.go - mount validation simplified from 46 lines to 5 lines

Impact

Eliminates ~120 lines of duplicated validation:

3 port validation implementations → 1
4 timeout validation implementations → 1
2 mount validation implementations → 1

Example of the consolidation:

// Before: duplicated in 3 files
if port < 1 || port > 65535 {
    return &ValidationError{
        Field: "port",
        Message: fmt.Sprintf("port must be between 1 and 65535, got %d", port),
        JSONPath: "gateway.port",
        Suggestion: "Use a valid port number (e.g., 8080)",
    }
}

// After: single implementation
if err := rules.PortRange(port, "gateway.port"); err != nil {
    return err
}

Backward compatibility maintained via type alias in validation.go.

Original prompt

This section details on the original issue you should resolve

<issue_title>[duplicate-code] Config Validation Logic Duplication (Medium Severity)</issue_title>
<issue_description>## Summary
Config validation has three separate validation files with overlapping responsibilities and duplicated validation patterns for ports, timeouts, and field constraints.

Duplication Details

Pattern 1: Port Range Validation (3 instances)

Files:

internal/config/env_validation.go:275-276
internal/config/schema_validation.go:240-248
internal/config/validation.go:201-210

Duplicated Code:

// env_validation.go:275-276
if port < 1 || port > 65535 {
	return 0, fmt.Errorf("MCP_GATEWAY_PORT must be between 1 and 65535, got %d", port)
}

// schema_validation.go:240-248
if port < 1 || port > 65535 {
	return &ValidationError{
		Field:      "port",
		Message:    fmt.Sprintf("port must be between 1 and 65535, got %d", port),
		JSONPath:   "gateway.port",
		Suggestion: "Use a valid port number (e.g., 8080)",
	}
}

// validation.go:201-210
if *gateway.Port < 1 || *gateway.Port > 65535 {
	return &ValidationError{
		Field:      "port",
		Message:    fmt.Sprintf("port must be between 1 and 65535, got %d", *gateway.Port),
		JSONPath:   "gateway.port",
		Suggestion: "Use a valid port number (e.g., 8080)",
	}
}

Pattern 2: Timeout Validation (4 instances)

Files:

internal/config/schema_validation.go:265-281
internal/config/validation.go:213-229

Duplicated Code:

// schema_validation.go:265-272
if stdinCfg.Gateway.StartupTimeout != nil && *stdinCfg.Gateway.StartupTimeout < 1 {
	return &ValidationError{
		Field:      "startupTimeout",
		Message:    fmt.Sprintf("startupTimeout must be at least 1, got %d", *stdinCfg.Gateway.StartupTimeout),
		JSONPath:   "gateway.startupTimeout",
		Suggestion: "Use a positive number of seconds (e.g., 30)",
	}
}

// validation.go:213-220 (identical pattern)

Both files repeat this for StartupTimeout and ToolTimeout.

Pattern 3: ValidationError Creation (15+ instances)

Pattern across both schema_validation.go and validation.go:

return &ValidationError{
	Field:      "fieldName",
	Message:    fmt.Sprintf("...", value),
	JSONPath:   "path.to.field",
	Suggestion: "...",
}

Instances:

schema_validation.go: 7 instances (lines 192, 203, 215, 226, 242, 266, 274)
validation.go: 8 instances (lines 49, 85, 96, 107, 117, 144, 154, 163)

Pattern 4: Mount Validation

Files:

internal/config/schema_validation.go:201-210
internal/config/validation.go:78-124

Both validate mount format source:dest:mode with nearly identical logic.

schema_validation.go:201-210

for i, mount := range server.Mounts {
	if !mountPattern.MatchString(mount) {
		return &ValidationError{
			Field:      "mounts",
			Message:    fmt.Sprintf("mount '%s' does not match required pattern", mount),
			JSONPath:   fmt.Sprintf("%s.mounts[%d]", jsonPath, i),
			Suggestion: "Use format 'source:dest:mode' where mode is 'ro' or 'rw'",
		}
	}
}

validation.go:78-124 (46 lines)

func validateMounts(mounts []string, jsonPath string) error {
	for i, mount := range mounts {
		parts := strings.Split(mount, ":")
		if len(parts) != 3 {
			return &ValidationError{
				Field:      "mounts",
				Message:    fmt.Sprintf("invalid mount format '%s' (expected 'source:dest:mode')", mount),
				JSONPath:   fmt.Sprintf("%s.mounts[%d]", jsonPath, i),
				Suggestion: "Use format 'source:dest:mode' where mode is 'ro' (read-only) or 'rw' (read-write)",
			}
		}

		source, dest, mode := parts[0], parts[1], parts[2]

		// Validate source is not empty
		if source == "" {
			return &ValidationError{...}
		}

		// Validate dest is not empty
		if dest == "" {
			return &ValidationError{...}
		}

		// Validate mode
		if mode != "ro" && mode != "rw" {
			return &ValidationError{...}
		}
	}
	return nil
}

File Responsibilities (Overlap)

env_validation.go

Environment variable validation
Docker accessibility checks
Container environment validation
PORT VALIDATION (overlap)

schema_validation.go

JSON schema validation
String pattern validation
PORT VALIDATION (overlap)
TIMEOUT VALIDATION (overlap)
MOUNT VALIDATION (overlap)

validation.go

Variable expansion
Server config validation
Gateway config validation
PORT VALIDATION (overlap)
TIMEOUT VALIDATION (overlap)
MOUNT VALIDATION (overlap)

Issues

1. Unclear Validation Flow

Three files validate the same fields
Unclear which validator runs when
Risk of conflicting validation logic

2. Inconsistent Error Messages

Port validation produces different error types:

env_validation: fmt.Errorf
schema_validation: &ValidationError
validation: &ValidationError

3. Maintenance Burden

Changing port range requires updating 3 files
Easy to miss one instance
No single source of truth

Severity Assessment: **ME...

Fixes [duplicate-code] Config Validation Logic Duplication (Medium Severity) #205

💡 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.

… files Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

Initial plan

c19ec4f

Copilot AI assigned Copilot and pelikhan Jan 13, 2026

Copilot started work on behalf of pelikhan January 13, 2026 01:21 View session

Create centralized validation rules package and update all validation…

1f92357

… files Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

Copilot AI changed the title ~~[WIP] Refactor config validation to remove duplicated logic~~ Consolidate duplicate config validation logic into centralized rules package Jan 13, 2026

Copilot AI requested a review from pelikhan January 13, 2026 01:29

Copilot finished work on behalf of pelikhan January 13, 2026 01:29

pelikhan marked this pull request as ready for review January 13, 2026 01:45

pelikhan merged commit 6420089 into main Jan 13, 2026
3 checks passed

pelikhan deleted the copilot/remove-duplicate-code-validation branch January 13, 2026 03:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Consolidate duplicate config validation logic into centralized rules package#208

Consolidate duplicate config validation logic into centralized rules package#208
pelikhan merged 2 commits intomainfrom
copilot/remove-duplicate-code-validation

Copilot AI commented Jan 13, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

Copilot AI commented Jan 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Impact

Duplication Details

Pattern 1: Port Range Validation (3 instances)

Pattern 2: Timeout Validation (4 instances)

Pattern 3: ValidationError Creation (15+ instances)

Pattern 4: Mount Validation

File Responsibilities (Overlap)

env_validation.go

schema_validation.go

validation.go

Issues

1. Unclear Validation Flow

2. Inconsistent Error Messages

3. Maintenance Burden

Severity Assessment: **ME...

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Copilot AI commented Jan 13, 2026 •

edited

Loading