Skip to content

Collapse duplicate unregistered custom-type handling in config validation#7043

Merged
lpcox merged 4 commits into
mainfrom
copilot/duplicate-code-fix
Jun 5, 2026
Merged

Collapse duplicate unregistered custom-type handling in config validation#7043
lpcox merged 4 commits into
mainfrom
copilot/duplicate-code-fix

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Jun 5, 2026

validateCustomServerConfig had two adjacent branches that produced the same rules.UnsupportedType(...) error for the same condition in practice: the custom type was not present in customSchemas. This change removes that duplication and keeps the unsupported custom-type error path defined in one place.

  • Validation flow

    • Remove the explicit customSchemas == nil early return from validateCustomServerConfig
    • Rely on Go's nil-map lookup semantics so both nil and missing-key cases fall through the same !exists branch
  • Behavior

    • Preserve the existing user-facing error for unregistered custom server types
    • Keep nil and missing-entry handling equivalent, but without duplicating the error construction
  • Coverage

    • Existing tests for both nil customSchemas and unregistered custom types continue to exercise the shared path
schemaValue, exists := customSchemas[serverType]
if !exists {
	return rules.UnsupportedType(
		"type",
		serverType,
		jsonPath,
		"Custom server type '"+serverType+"' is not registered in customSchemas. "+
			"Add the custom type to the customSchemas field or use a standard type ('stdio' or 'http')",
	)
}

Copilot AI changed the title [WIP] Fix duplicate code pattern in validateCustomServerConfig Collapse duplicate unregistered custom-type handling in config validation Jun 5, 2026
Copilot finished work on behalf of lpcox June 5, 2026 13:58
Copilot AI requested a review from lpcox June 5, 2026 13:58
@lpcox lpcox marked this pull request as ready for review June 5, 2026 14:39
Copilot AI review requested due to automatic review settings June 5, 2026 14:39
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR simplifies validateCustomServerConfig by removing duplicate logic for handling unregistered custom server types, relying on Go’s nil-map lookup behavior to unify the error path.

Changes:

  • Removed the explicit customSchemas == nil early return branch.
  • Consolidated nil map and missing-key handling into the existing !exists branch.
Show a summary per file
File Description
internal/config/validation.go Removes duplicate nil-check branch so unregistered custom types share one error path

Copilot's findings

Tip

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

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

Comment thread internal/config/validation.go Outdated
Comment on lines 199 to 202
// Check if custom type is registered
if customSchemas == nil {
logValidation.Printf("Custom type not registered: name=%s, type=%s (no customSchemas)", name, serverType)
return rules.UnsupportedType("type", serverType, jsonPath, "Custom server type '"+serverType+"' is not registered in customSchemas. Add the custom type to the customSchemas field or use a standard type ('stdio' or 'http')")
}

schemaValue, exists := customSchemas[serverType]
if !exists {
logValidation.Printf("Custom type not registered: name=%s, type=%s", name, serverType)
@lpcox
Copy link
Copy Markdown
Collaborator

lpcox commented Jun 5, 2026

@copilot address review feedback

Copy link
Copy Markdown
Contributor Author

Copilot AI commented Jun 5, 2026

@copilot address review feedback

Addressed in c4c35c9. I kept the unified unsupported-type error path and restored the nil-vs-missing distinction in logging by appending a (no customSchemas) suffix only when customSchemas is nil.

Copilot finished work on behalf of lpcox June 5, 2026 15:27
@lpcox lpcox merged commit b121fa1 into main Jun 5, 2026
16 checks passed
@lpcox lpcox deleted the copilot/duplicate-code-fix branch June 5, 2026 15:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[duplicate-code] Duplicate Code Pattern: Identical rules.UnsupportedType call in validateCustomServerConfig

3 participants