Deduplicate shared config validation across TOML and stdin paths by Copilot · Pull Request #7885 · github/gh-aw-mcpg · GitHub
Skip to content

Deduplicate shared config validation across TOML and stdin paths#7885

Merged
lpcox merged 4 commits into
mainfrom
copilot/duplicate-code-config-validation
Jun 21, 2026
Merged

Deduplicate shared config validation across TOML and stdin paths#7885
lpcox merged 4 commits into
mainfrom
copilot/duplicate-code-config-validation

Conversation

Copilot AI commented Jun 21, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Server-level config validation had drift-prone duplication between the TOML loader and JSON stdin validation flow. The same auth, tool_response_filters, and gateway payload threshold rules were implemented in parallel, with parity enforced only by convention and comments.

  • Shared validation helpers

    • Extracted validateCommonServerFields(...) to centralize per-server auth and tool_response_filters validation.
    • Extracted validateGatewayPayloadSizeThreshold(...) to centralize positive-integer validation for gateway payload threshold fields across both config formats.

  • TOML path

    • Updated LoadFromFile to delegate shared per-server validation to the new helper.
    • Replaced inline payload_size_threshold validation with the shared gateway helper.

  • JSON stdin path

    • Updated validateStandardServerConfig to use the shared per-server helper instead of revalidating auth and response filters inline.
    • Updated validateGatewayConfig to use the shared gateway payload-threshold helper.

  • Regression coverage

    • Added a parity test asserting TOML loading and stdin validation produce the same error for the same invalid tool_response_filters input.
func validateCommonServerFields(name, serverType string, auth *AuthConfig, filters map[string]string, jsonPath string) error {
	if err := validateServerAuth(auth, serverType, name, jsonPath); err != nil {
		return err
	}
	return validateToolResponseFilters(filters, jsonPath+".tool_response_filters")
}

GitHub Advanced Security started work on behalf of lpcox June 21, 2026 16:09 View session
Copilot AI changed the title [WIP] Refactor duplicated config validation logic for TOML and JSON Deduplicate shared config validation across TOML and stdin paths Jun 21, 2026
Copilot AI requested a review from lpcox June 21, 2026 16:10
Copilot finished work on behalf of lpcox June 21, 2026 16:10
GitHub Advanced Security finished work on behalf of lpcox June 21, 2026 16:11
GitHub Advanced Security started work on behalf of lpcox June 21, 2026 16:18 View session
GitHub Advanced Security finished work on behalf of lpcox June 21, 2026 16:20
@lpcox lpcox marked this pull request as ready for review June 21, 2026 17:09
Copilot AI review requested due to automatic review settings June 21, 2026 17:09
Copilot AI reviewed Jun 21, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR reduces drift risk in configuration validation by centralizing shared validation logic used by both the TOML loader (LoadFromFile) and the JSON-stdin validation flow, and adds a regression test intended to ensure parity for invalid tool_response_filters.

Changes:

  • Introduces validateCommonServerFields(...) to share per-server auth and tool_response_filters validation across TOML and stdin paths.
  • Introduces validateGatewayPayloadSizeThreshold(...) and updates both config formats to use it for gateway payload threshold validation.
  • Adds a parity regression test for invalid tool_response_filters jq expressions.
Show a summary per file
File Description
internal/config/validation.go Adds shared validation helpers and switches stdin gateway validation to use the shared payload threshold validator.
internal/config/config_core.go Switches TOML load-time per-server validation and payload_size_threshold validation to the shared helpers.
internal/config/config_core_test.go Adds a regression test intended to assert parity between TOML and stdin validation errors for invalid jq filters.

Copilot's findings

Tip

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

  • Files reviewed: 3/3 changed files
  • Comments generated: 1

Comment thread internal/config/config_core_test.go Outdated
@lpcox
Potential fix for pull request finding
5206859 
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
GitHub Advanced Security started work on behalf of lpcox June 21, 2026 17:14 View session
GitHub Advanced Security finished work on behalf of lpcox June 21, 2026 17:16
@lpcox

lpcox commented Jun 21, 2026

Copy link
Copy Markdown
Collaborator

@copilot fix failing ci checks

1 similar comment
@lpcox

lpcox commented Jun 21, 2026

Copy link
Copy Markdown
Collaborator

@copilot fix failing ci checks

Copilot AI commented Jun 21, 2026

Copy link
Copy Markdown
Contributor Author

@copilot fix failing ci checks

Fixed in the latest commit. There were three separate issues causing CI failures:

  1. Syntax error — TestLoadFromFile_ToolResponseFilter_InvalidJqExpressionMatchesStdinValidation was missing its closing }, which the Go parser reported as expected '(', found TestLoadFromFile_ToolResponseFilter_WhitespaceOnlyValue.

  2. Duplicate TLS files — tls_config.go and tls_config_test.go were accidentally introduced alongside the existing tls.go/tls_test.go, redeclaring MinTLSVersion, NewServerTLSConfig, and NewClientTLSConfig. Removed the duplicates and restored tls_test.go to match main.

  3. Nil interface bug in validateGatewayPayloadSizeThreshold — PositiveInteger returns *ValidationError. Returning it directly as error produced a non-nil interface even when the pointer was nil, causing valid thresholds (≥ 1) to be incorrectly flagged as errors. Fixed by explicitly returning nil when the concrete pointer is nil.

GitHub Advanced Security started work on behalf of lpcox June 21, 2026 17:24 View session
Copilot finished work on behalf of lpcox June 21, 2026 17:24
GitHub Advanced Security finished work on behalf of lpcox June 21, 2026 17:25

Copilot AI commented Jun 21, 2026

Copy link
Copy Markdown
Contributor Author

Copilot finished work on behalf of lpcox June 21, 2026 17:31
@lpcox lpcox merged commit b1296c6 into main Jun 21, 2026
28 checks passed
@lpcox lpcox deleted the copilot/duplicate-code-config-validation branch June 21, 2026 17:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments
@lpcox lpcox Awaiting requested review from lpcox

Assignees

@lpcox lpcox

Copilot code review Copilot

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@lpcox