Skip to content

Consolidate duplicate SetGatewayVersion implementations #695

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
lpcox merged 4 commits into from
Feb 5, 2026
Merged

Consolidate duplicate SetGatewayVersion implementations #695

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6340dba
Initial plan
Copilot Feb 5, 2026
d3d946f
Create unified version package and refactor version management
Copilot Feb 5, 2026
f8fb136
Complete version consolidation refactoring - all tests pass
Copilot Feb 5, 2026
9872d99
Address code review comments - add documentation
Copilot Feb 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 10 additions & 10 deletions internal/cmd/root.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ import (

"github.com/github/gh-aw-mcpg/internal/config"
"github.com/github/gh-aw-mcpg/internal/logger"
"github.com/github/gh-aw-mcpg/internal/mcp"
"github.com/github/gh-aw-mcpg/internal/server"
"github.com/github/gh-aw-mcpg/internal/version"
"github.com/spf13/cobra"
)

Expand All @@ -32,13 +32,17 @@ const (
// Package-level variables that don't belong to a specific feature
var (
debugLog = logger.New("cmd:root")
version = "dev" // Default version, overridden by SetVersion
// cliVersion stores the version string for Cobra's CLI version display.
// This is kept separate from version.Get() because rootCmd.Version must be
// set at initialization time (before SetVersion is called). We sync both
// values in SetVersion() to maintain a single source of truth.
cliVersion = "dev" // Default version, overridden by SetVersion
)

var rootCmd = &cobra.Command{
Use: "awmg",
Short: "MCPG MCP proxy server",
Version: version,
Version: cliVersion,
Long: `MCPG is a proxy server for Model Context Protocol (MCP) servers.
It provides routing, aggregation, and management of multiple MCP backend servers.`,
SilenceUsage: true, // Don't show help on runtime errors
Expand Down Expand Up @@ -146,7 +150,7 @@ func run(cmd *cobra.Command, args []string) error {
}
defer logger.CloseJSONLLogger()

logger.LogInfoMd("startup", "MCPG Gateway version: %s", version)
logger.LogInfoMd("startup", "MCPG Gateway version: %s", cliVersion)

// Log config source based on what was provided
configSource := configFile
Expand Down Expand Up @@ -232,9 +236,6 @@ func run(cmd *cobra.Command, args []string) error {

debugLog.Printf("Server mode: %s, DIFC enabled: %v", mode, cfg.EnableDIFC)

// Set gateway version for health endpoint reporting
server.SetGatewayVersion(version)

// Create unified MCP server (backend for both modes)
unifiedServer, err := server.NewUnified(ctx, cfg)
if err != nil {
Expand Down Expand Up @@ -449,8 +450,7 @@ func Execute() {

// SetVersion sets the version string for the CLI
func SetVersion(v string) {
version = v
cliVersion = v
rootCmd.Version = v
config.SetGatewayVersion(v)
mcp.SetClientGatewayVersion(v)
version.Set(v)
}
15 changes: 3 additions & 12 deletions internal/config/validation_schema.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (

"github.com/github/gh-aw-mcpg/internal/config/rules"
"github.com/github/gh-aw-mcpg/internal/logger"
"github.com/github/gh-aw-mcpg/internal/version"
"github.com/santhosh-tekuri/jsonschema/v5"
)

Expand All @@ -22,9 +23,6 @@ var (
mountPattern = regexp.MustCompile(`^[^:]+:[^:]+(:(ro|rw))?$`)
domainVarPattern = regexp.MustCompile(`^\$\{[A-Z_][A-Z0-9_]*\}$`)

// gatewayVersion stores the version string to include in error messages
gatewayVersion = "dev"

// logSchema is the debug logger for schema validation
logSchema = logger.New("config:validation_schema")

Expand All @@ -49,13 +47,6 @@ var (
schemaErr error
)

// SetGatewayVersion sets the gateway version for error reporting
func SetGatewayVersion(version string) {
if version != "" {
gatewayVersion = version
}
}

// fetchAndFixSchema fetches the JSON schema from the remote URL and applies
// workarounds for JSON Schema Draft 7 limitations.
//
Expand Down Expand Up @@ -265,7 +256,7 @@ func formatSchemaError(err error) error {
// The jsonschema library returns a ValidationError type with detailed info
if ve, ok := err.(*jsonschema.ValidationError); ok {
var sb strings.Builder
sb.WriteString(fmt.Sprintf("Configuration validation error (MCP Gateway version: %s):\n\n", gatewayVersion))
sb.WriteString(fmt.Sprintf("Configuration validation error (MCP Gateway version: %s):\n\n", version.Get()))

// Recursively format all errors
formatValidationErrorRecursive(ve, &sb, 0)
Expand All @@ -275,7 +266,7 @@ func formatSchemaError(err error) error {
return fmt.Errorf("%s", sb.String())
}

return fmt.Errorf("configuration validation error (version: %s): %s", gatewayVersion, err.Error())
return fmt.Errorf("configuration validation error (version: %s): %s", version.Get(), err.Error())
}

// formatValidationErrorRecursive recursively formats validation errors with proper indentation
Expand Down
3 changes: 2 additions & 1 deletion internal/config/validation_schema_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"strings"
"testing"

"github.com/github/gh-aw-mcpg/internal/version"
"github.com/stretchr/testify/assert"
)

Expand Down Expand Up @@ -528,7 +529,7 @@ func TestValidateStringPatterns(t *testing.T) {
// TestEnhancedErrorMessages verifies that validation errors include version and detailed context
func TestEnhancedErrorMessages(t *testing.T) {
// Set a test version
SetGatewayVersion("v1.2.3-test")
version.Set("v1.2.3-test")

tests := []struct {
name string
Expand Down
14 changes: 2 additions & 12 deletions internal/mcp/connection.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,22 +17,12 @@ import (

"github.com/github/gh-aw-mcpg/internal/logger"
"github.com/github/gh-aw-mcpg/internal/logger/sanitize"
"github.com/github/gh-aw-mcpg/internal/version"
sdk "github.com/modelcontextprotocol/go-sdk/mcp"
)

var logConn = logger.New("mcp:connection")

// gatewayVersion stores the gateway version used in MCP client implementation
// It defaults to "dev" and is set at startup via SetClientGatewayVersion
var gatewayVersion = "dev"

// SetClientGatewayVersion sets the gateway version for MCP client implementation reporting
func SetClientGatewayVersion(version string) {
if strings.TrimSpace(version) != "" {
gatewayVersion = version
}
}

// parseSSEResponse extracts JSON data from SSE-formatted response
// SSE format: "event: message\ndata: {json}\n\n"
func parseSSEResponse(body []byte) ([]byte, error) {
Expand Down Expand Up @@ -88,7 +78,7 @@ type Connection struct {
func newMCPClient() *sdk.Client {
return sdk.NewClient(&sdk.Implementation{
Name: "awmg",
Version: gatewayVersion,
Version: version.Get(),
}, nil)
}

Expand Down
3 changes: 2 additions & 1 deletion internal/server/health.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"net/http"

"github.com/github/gh-aw-mcpg/internal/logger"
"github.com/github/gh-aw-mcpg/internal/version"
)

var logHealth = logger.New("server:health")
Expand Down Expand Up @@ -40,7 +41,7 @@ func BuildHealthResponse(unifiedServer *UnifiedServer) HealthResponse {
return HealthResponse{
Status: overallStatus,
SpecVersion: MCPGatewaySpecVersion,
GatewayVersion: gatewayVersion,
GatewayVersion: version.Get(),
Servers: serverStatus,
}
}
Expand Down
3 changes: 2 additions & 1 deletion internal/server/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"github.com/github/gh-aw-mcpg/internal/logger"
"github.com/github/gh-aw-mcpg/internal/mcp"
"github.com/github/gh-aw-mcpg/internal/sys"
"github.com/github/gh-aw-mcpg/internal/version"
)

var logServer = logger.New("server:server")
Expand Down Expand Up @@ -61,7 +62,7 @@ func (s *Server) handleHealth(w http.ResponseWriter, r *http.Request) {
json.NewEncoder(w).Encode(map[string]string{
"status": "ok",
"protocolVersion": MCPProtocolVersion,
"version": gatewayVersion,
"version": version.Get(),
})
}

Expand Down
10 changes: 0 additions & 10 deletions internal/server/unified.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,16 +29,6 @@ const MCPProtocolVersion = "2024-11-05"
// MCPGatewaySpecVersion is the MCP Gateway Specification version this implementation conforms to
const MCPGatewaySpecVersion = "1.5.0"

// gatewayVersion stores the gateway version, set at startup
var gatewayVersion = "dev"

// SetGatewayVersion sets the gateway version for health endpoint reporting
func SetGatewayVersion(version string) {
if version != "" {
gatewayVersion = version
}
}

// Session represents a MCPG session
type Session struct {
Token string
Expand Down
23 changes: 23 additions & 0 deletions internal/version/version.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package version

// gatewayVersion stores the gateway version string, used across multiple packages
// for error reporting, health checks, and MCP client implementation info.
// It defaults to "dev" and should be set once at startup.
//
// Thread-safety note: This variable is written once at application startup
// (in SetVersion) before any concurrent access, and read-only thereafter.
Copy link

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

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

The comment mentions "in SetVersion" but the function is actually named "Set". This should be corrected to "in Set()" for accuracy.

Suggested change
// (in SetVersion) before any concurrent access, and read-only thereafter.
// (in Set()) before any concurrent access, and read-only thereafter.

Copilot uses AI. Check for mistakes.
// No mutex is needed as the write happens before any goroutines are spawned.
var gatewayVersion = "dev"

// Set updates the gateway version string if the provided version is non-empty.
// This should be called once at application startup from main.
func Set(v string) {
if v != "" {
gatewayVersion = v
}
Comment on lines +14 to +17
Copy link

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

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

The original mcp.SetClientGatewayVersion implementation used strings.TrimSpace(version) != "" to check for empty strings, while config.SetGatewayVersion and server.SetGatewayVersion used version != "". The new implementation follows the config/server pattern, which means it will accept whitespace-only strings as valid versions (unlike the original mcp implementation). While unlikely to cause issues since the version is set from properly formatted strings in main.go, this is a subtle behavior change. Consider using strings.TrimSpace(v) != "" to maintain the most restrictive behavior from all three original implementations.

Copilot uses AI. Check for mistakes.
}

// Get returns the current gateway version string.
func Get() string {
return gatewayVersion
}
69 changes: 69 additions & 0 deletions internal/version/version_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
package version

import (
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestSet(t *testing.T) {
tests := []struct {
name string
inputVersion string
expectedResult string
}{
{
name: "set valid version",
inputVersion: "v1.2.3",
expectedResult: "v1.2.3",
},
{
name: "set version with build metadata",
inputVersion: "v1.2.3, commit: abc1234, built: 2024-01-01",
expectedResult: "v1.2.3, commit: abc1234, built: 2024-01-01",
},
{
name: "empty string does not change version",
inputVersion: "",
expectedResult: "dev", // should remain default
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
// Reset to default before each test
gatewayVersion = "dev"

Set(tt.inputVersion)
result := Get()
assert.Equal(t, tt.expectedResult, result)
})
}
}

func TestGet(t *testing.T) {
// Reset to default
gatewayVersion = "dev"

// Test default value
result := Get()
require.Equal(t, "dev", result, "Default version should be 'dev'")

// Test after setting a value
Set("v2.0.0")
result = Get()
assert.Equal(t, "v2.0.0", result, "Version should be updated to 'v2.0.0'")
}

func TestSetPreservesVersionOnEmpty(t *testing.T) {
// Set an initial version
gatewayVersion = "v1.0.0"

// Try to set empty string
Set("")

// Version should remain unchanged
result := Get()
assert.Equal(t, "v1.0.0", result, "Version should not change when empty string is provided")
}
Loading