refactor: consolidate environment variable reading using envutil helpers

[Claude]

internal/config/config_env.go duplicated environment variable reading logic that already exists in internal/envutil. Three functions used direct os.Getenv() calls instead of the standard envutil.GetEnvString() utility.

Changes

• Replaced GetGatewayDomainFromEnv() and GetGatewayAPIKeyFromEnv() with single-line envutil.GetEnvString() calls
• Updated GetGatewayPortFromEnv() to use envutil.GetEnvString() for raw environment variable read while preserving port validation logic
• Removed unused os import, added envutil import

Before/After

// Before: Direct os.Getenv calls
func GetGatewayDomainFromEnv() string {
    return os.Getenv("MCP_GATEWAY_DOMAIN")
}

// After: Consistent with codebase pattern
func GetGatewayDomainFromEnv() string {
    return envutil.GetEnvString("MCP_GATEWAY_DOMAIN", "")
}

Behavior unchanged - all existing tests pass without modification.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

• example.com
  • Triggering command: /tmp/go-build3717154652/b275/launcher.test /tmp/go-build3717154652/b275/launcher.test -test.testlogfile=/tmp/go-build3717154652/b275/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true -unreachable=false din}} x_amd64/vet --short (dns block)
• invalid-host-that-does-not-exist-12345.com
  • Triggering command: /tmp/go-build289446620/b001/config.test /tmp/go-build289446620/b001/config.test -test.testlogfile=/tmp/go-build289446620/b001/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true go k1BRF76vb .12/x64/as (dns block)
• nonexistent.local
  • Triggering command: /tmp/go-build3717154652/b275/launcher.test /tmp/go-build3717154652/b275/launcher.test -test.testlogfile=/tmp/go-build3717154652/b275/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true -unreachable=false din}} x_amd64/vet --short (dns block)
• slow.example.com
  • Triggering command: /tmp/go-build3717154652/b275/launcher.test /tmp/go-build3717154652/b275/launcher.test -test.testlogfile=/tmp/go-build3717154652/b275/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true -unreachable=false din}} x_amd64/vet --short (dns block)
• this-host-does-not-exist-12345.com
  • Triggering command: /tmp/go-build3717154652/b284/mcp.test /tmp/go-build3717154652/b284/mcp.test -test.testlogfile=/tmp/go-build3717154652/b284/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true -unreachable=false /tmp/go-build1892238626/b145/vet.cfg x_amd64/vet --short (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

• Configure Actions setup steps to set up my environment, which run before the firewall is enabled
• Add the appropriate URLs or hosts to the custom allowlist in this repository's Copilot coding agent settings (admins only)

Original prompt

---

This section details on the original issue you should resolve

<issue_title>[duplicate-code] Duplicate Code Pattern: config/config_env.go Reimplements envutil Functionality</issue_title>
<issue_description>Part of duplicate code analysis: #1218

Summary

internal/config/config_env.go reads environment variables using os.Getenv directly with manual strconv.Atoi parsing, while the rest of the codebase uses the internal/envutil package for the same operations. GetGatewayDomainFromEnv and GetGatewayAPIKeyFromEnv are trivial os.Getenv wrappers that duplicate what envutil.GetEnvString already provides. GetGatewayPortFromEnv reimplements integer parsing logic that envutil.GetEnvInt already handles, but adds port-range validation on top — mixing concerns.

Duplication Details

Pattern: Manual env-var reading duplicating envutil helpers

• Severity: Medium

• Occurrences: 3 functions in config_env.go vs equivalent patterns in envutil

• Locations:

  • internal/config/config_env.go (lines 1–38): all three getter functions
  • internal/envutil/envutil.go (lines 9–43): GetEnvString, GetEnvInt, GetEnvBool

• Code Sample:

// config/config_env.go — manual approach
func GetGatewayPortFromEnv() (int, error) {
    portStr := os.Getenv("MCP_GATEWAY_PORT")
    if portStr == "" {
        return 0, fmt.Errorf("MCP_GATEWAY_PORT environment variable not set")
    }
    port, err := strconv.Atoi(portStr)
    if err != nil {
        return 0, fmt.Errorf("invalid MCP_GATEWAY_PORT value: %s", portStr)
    }
    if validationErr := rules.PortRange(port, "MCP_GATEWAY_PORT"); validationErr != nil {
        return 0, fmt.Errorf("%s", validationErr.Message)
    }
    return port, nil
}

func GetGatewayDomainFromEnv() string { return os.Getenv("MCP_GATEWAY_DOMAIN") }
func GetGatewayAPIKeyFromEnv() string { return os.Getenv("MCP_GATEWAY_API_KEY") }

// envutil/envutil.go — existing utility
func GetEnvString(envKey, defaultValue string) string { ... }
func GetEnvInt(envKey string, defaultValue int) int { ... }

GetGatewayDomainFromEnv and GetGatewayAPIKeyFromEnv are pure one-line os.Getenv wrappers that could simply be envutil.GetEnvString("MCP_GATEWAY_DOMAIN", ""). GetGatewayPortFromEnv adds error-return semantics and port validation that envutil.GetEnvInt doesn't provide, making it partially legitimate — but the int-parsing logic is duplicated.

Impact Analysis

• Maintainability: Medium — two different patterns for reading env vars creates confusion for contributors adding new env vars. They must decide which approach to follow
• Bug Risk: Low-medium — envutil.GetEnvInt silently returns the default if parsing fails; GetGatewayPortFromEnv returns an error. These behave differently, which can cause subtle inconsistencies if future callers mix approaches
• Code Bloat: ~15 lines in config_env.go that could be replaced with envutil calls or a thin validation wrapper

Refactoring Recommendations

1. Replace GetGatewayDomainFromEnv and GetGatewayAPIKeyFromEnv with envutil.GetEnvString

   • These are trivially replaceable: callers get the same behaviour
   • Estimated effort: < 15 minutes
   • Benefits: Consistent env-var reading pattern, removes two redundant functions

2. Refactor GetGatewayPortFromEnv to separate parsing from validation

   • Keep the error-returning signature (needed by callers that require a port)
   • Use envutil.GetEnvString to read the raw value, then apply strconv.Atoi + rules.PortRange
   • This keeps the validation logic without duplicating the string-reading pattern

// Suggested refactor
func GetGatewayPortFromEnv() (int, error) {
    portStr := envutil.GetEnvString("MCP_GATEWAY_PORT", "")
    if portStr == "" {
        return 0, fmt.Errorf("MCP_GATEWAY_PORT environment variable not set")
    }
    port, err := strconv.Atoi(portStr)
    if err != nil {
        return 0, fmt.Errorf("invalid MCP_GATEWAY_PORT value: %s", portStr)
    }
    if validationErr := rules.PortRange(port, "MCP_GATEWAY_PORT"); validationErr != nil {
        return 0, fmt.Errorf("%s", validationErr.Message)
    }
    return port, nil
}

func GetGatewayDomainFromEnv() string { return envutil.GetEnvString("MCP_GATEWAY_DOMAIN", "") }
func GetGatewayAPIKeyFromEnv() string  { return envutil.GetEnvString("MCP_GATEWAY_API_KEY", "") }

Implementation Checklist

• Replace GetGatewayDomainFromEnv body with envutil.GetEnvString("MCP_GATEWAY_DOMAIN", "")
• Replace GetGatewayAPIKeyFromEnv body with envutil.GetEnvString("MCP_GATEWAY_API_KEY", "")
• Optionally update GetGatewayPortFromEnv to use envutil.GetEnvString for the raw read
• Add envutil import to config_env.go
• Verify tests still pass (make test)

Parent Issue

See parent analysis report: #1218
Related to #1218

Generated by [Duplicate Code Detecto...

• Fixes [duplicate-code] Duplicate Code Pattern: config/config_env.go Reimplements envutil Functionality #1220

[Copilot]

Pull request overview

This PR refactors environment variable reading in internal/config/config_env.go to use the standard envutil helper functions instead of direct os.Getenv() calls. This consolidates duplicate environment variable reading logic and ensures consistency with the rest of the codebase.

Changes:

• Replaced direct os.Getenv() calls with envutil.GetEnvString() in three functions
• Removed unused os import and added envutil import
• Preserved all existing behavior including port validation logic

---

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