refactor(config): extract shared validateServerAuth to deduplicate auth validation#3764
Merged
refactor(config): extract shared validateServerAuth to deduplicate auth validation#3764
Conversation
…th validation The TOML path (LoadFromFile in config_core.go) and JSON stdin path (validateStandardServerConfig in validation.go) both performed the same auth validation: reject auth on non-HTTP servers, then call validateAuthConfig. Extract a shared validateServerAuth helper that handles nil-check, type-check, and delegation in one place. - config_core.go: replaced 8-line inline loop body with 1-line call - validation.go (stdio block): replaced structured error with shared helper - validation.go (http block): replaced inline validateAuthConfig with shared helper Fixes #3561 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Closed
6 tasks
Contributor
There was a problem hiding this comment.
Pull request overview
Refactors configuration validation to deduplicate per-server auth validation across the TOML loader (LoadFromFile) and the JSON-stdin validation path, by introducing a shared validateServerAuth(...) helper.
Changes:
- Added
validateServerAuth(auth, serverType, name, jsonPath)helper ininternal/config/validation.go. - Updated JSON-stdin standard server validation to call the shared helper instead of inline auth checks.
- Updated TOML
LoadFromFileauth validation loop to delegate to the shared helper.
Show a summary per file
| File | Description |
|---|---|
| internal/config/validation.go | Introduces validateServerAuth and replaces duplicated auth validation logic in validateStandardServerConfig. |
| internal/config/config_core.go | Replaces inline auth validation in LoadFromFile with a call to validateServerAuth. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 2/2 changed files
- Comments generated: 2
Comment on lines
+244
to
+248
| // validateServerAuth validates the auth configuration on any server type, | ||
| // rejecting auth on non-HTTP servers and delegating to validateAuthConfig | ||
| // for HTTP servers. This is shared by both the TOML (LoadFromFile) and | ||
| // JSON stdin (validateStandardServerConfig) paths. | ||
| func validateServerAuth(auth *AuthConfig, serverType, name, jsonPath string) error { |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
lpcox
added a commit
that referenced
this pull request
Apr 14, 2026
PR #3764 refactored auth validation into validateServerAuth which uses rules.UnsupportedField, changing the error message format. Four tests still expected the old "auth is only supported for HTTP servers" wording. Updated assertions to match the actual structured error messages. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
lpcox
added a commit
that referenced
this pull request
Apr 14, 2026
…#3778) ## Problem PR #3764 refactored auth validation into a shared `validateServerAuth` helper that uses `rules.UnsupportedField` for structured errors. This changed the error message format from the old `"auth is only supported for HTTP servers"` to a structured `"server type \"stdio\""` with a suggestion. Four tests were not updated to match the new format: - `TestLoadFromFile_AuthOnNonHTTPServerRejected` — expected `"HTTP"` - `TestConvertStdinServerConfig_ValidationError/stdio_with_auth_block` — expected old message - `TestConvertStdinServerConfig_StdioWithAuth` — expected old message in `valErr.Message` - `TestValidateAuthConfig/auth_on_stdio_server_is_rejected` — expected old message ## Fix Updated all 4 assertions to match the actual structured error messages from `rules.UnsupportedField`.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #3561
Problem
Auth validation logic was duplicated between the TOML path (
LoadFromFileinconfig_core.go) and the JSON stdin path (validateStandardServerConfiginvalidation.go). Both independently checked that auth is only on HTTP servers, then calledvalidateAuthConfig.Solution
Extracted a shared
validateServerAuth(auth, serverType, name, jsonPath)helper that:validateAuthConfigfor HTTP serversBoth paths now call this single helper instead of duplicating the logic.
Changes
config_core.govalidateServerAuthcallvalidation.go(stdio)rules.UnsupportedFielderrorvalidation.go(http)validateAuthConfigcallAll existing tests pass unchanged.