refactor: semantic function clustering improvements (Finding 1–4) (#8507)

Addresses all 4 actionable findings from the automated semantic analysis
of 146 non-test Go source files
([#8499](#8499)).

## Finding 1 — `PositiveInteger` delegates to `TimeoutMinimum`
`internal/config/validation_rules.go` — eliminates duplicated `< 1`
guard; matches the pattern already used by `TimeoutPositive`. Error
`Message` and `Suggestion` text preserved via override:
```go
// Before: independent < 1 guard
if value < 1 { return &ValidationError{...} }

// After: delegates, then customises the returned error
if err := TimeoutMinimum(value, 1, fieldName, jsonPath); err != nil {
    err.Message    = fmt.Sprintf("%s must be a positive integer (>= 1), got %d", ...)
    err.Suggestion = fmt.Sprintf("Use a positive integer (>= 1) for %s", ...)
    return err
}
```

## Finding 2 — `internal/strutil` → `internal/util`
Package renamed to `util`: the package contains duration formatting,
cryptographic randomness, JSON deep-clone, and collection utilities —
none of which are string-specific. All ~26 import sites updated
(production, tests, AGENTS.md, CONTRIBUTING.md).

## Finding 3 — Logger factory functions inlined
`file_logger.go`, `markdown_logger.go`, `jsonl_logger.go`,
`tools_logger.go`, `observed_url_domains_logger.go` — 10 named functions
(`setupX` / `handleXError`, 2 per logger × 5 loggers) that were each
referenced exactly once as struct field values are inlined as anonymous
functions directly in their `loggerFactory` var declarations. Tests
updated to use `factory.setup(...)` / `factory.onError(...)`.

## Finding 4 — Route arg-key constants in `proxy/router.go`
Seven constants (`argOwner`, `argRepo`, `argPullNumber`,
`argIssueNumber`, `argMethod`, `argResourceID`, `argDiscussionNumber`)
replace ~20 scattered bare string literals across the route table and
`MatchRoute`, providing a single source of truth and typo protection.

Author: lpcox

AGENTS.md

@@ -45,7 +45,7 @@ Quick reference for AI agents working with MCP Gateway (Go-based MCP proxy serve
 - `internal/proxy/` - Filtering HTTP proxy for the GitHub API with DIFC enforcement
 - `internal/sanitize/` - Sensitive data redaction utilities (`SanitizeString`, `SanitizeJSON`, `TruncateSecret`) for safe log output
 - `internal/server/` - HTTP server (routed/unified modes)
-- `internal/strutil/` - String and formatting utilities
+- `internal/util/` - String, formatting, randomness, and JSON deep-clone utilities
 - `internal/syncutil/` - Concurrency utilities
 - `internal/sys/` - System utilities
 - `internal/testutil/` - Test utilities and helpers

CONTRIBUTING.md

@@ -296,7 +296,7 @@ gh-aw-mcpg/
     ├── proxy/                 # HTTP forward proxy for DIFC filtering
     ├── sanitize/              # Sensitive data redaction utilities for logging
     ├── server/                # HTTP server (routed/unified modes)
-    ├── strutil/               # String and formatting utility helpers (deduplication, trimming, duration formatting, JSON deep-clone)
+    ├── util/               # String, formatting, randomness, and JSON deep-clone utilities
     ├── syncutil/              # Concurrency utility helpers
     ├── sys/                   # System utilities
     ├── testutil/              # Test utilities and helpers
@@ -325,7 +325,7 @@ gh-aw-mcpg/
 - **`internal/proxy/`** - HTTP forward proxy applying DIFC filtering to `gh` CLI and REST/GraphQL requests
 - **`internal/sanitize/`** - Sensitive data redaction utilities (`SanitizeString`, `SanitizeJSON`, `TruncateSecret`) for safe log output
 - **`internal/server/`** - HTTP server with routed and unified modes
-- **`internal/strutil/`** - String and formatting utility helpers (deduplication, trimming, duration formatting, JSON deep-clone)
+- **`internal/util/`** - String, formatting, randomness, and JSON deep-clone utilities
 - **`internal/syncutil/`** - Concurrency utility helpers (get-or-create pattern)
 - **`internal/sys/`** - System utilities
 - **`internal/testutil/`** - Test utilities and helpers

internal/auth/header.go

@@ -41,7 +41,7 @@ import (
 
 	"github.com/github/gh-aw-mcpg/internal/logger"
 	"github.com/github/gh-aw-mcpg/internal/sanitize"
-	"github.com/github/gh-aw-mcpg/internal/strutil"
+	"github.com/github/gh-aw-mcpg/internal/util"
 )
 
 var log = logger.New("auth:header")
@@ -206,7 +206,7 @@ func IsMalformedHeader(header string) bool {
 // if none is provided. Returns a 32-byte hex-encoded string (64 chars).
 func GenerateRandomAgentID() (string, error) {
 	logAPIKey.Print("Generating random agent ID")
-	key, err := strutil.RandomHex(32)
+	key, err := util.RandomHex(32)
 	if err != nil {
 		logAPIKey.Printf("Random agent ID generation failed: %v", err)
 		return "", fmt.Errorf("failed to generate random agent ID: %w", err)

internal/config/guard_policy_validation.go

@@ -6,7 +6,7 @@ import (
 	"sort"
 	"strings"
 
-	"github.com/github/gh-aw-mcpg/internal/strutil"
+	"github.com/github/gh-aw-mcpg/internal/util"
 )
 
 const errMsgPolicyMissingKey = "policy must include allow-only or write-sink"
@@ -188,7 +188,7 @@ func NormalizeGuardPolicy(policy *GuardPolicy) (*NormalizedGuardPolicy, error) {
 		return normalized, nil
 
 	case []string:
-		scopes, err := normalizeAndValidateScopeArray(strutil.StringsToAny(scope))
+		scopes, err := normalizeAndValidateScopeArray(util.StringsToAny(scope))
 		if err != nil {
 			return nil, err
 		}

internal/config/validation_rules.go

@@ -46,16 +46,16 @@ func TimeoutPositive(timeout int, fieldName, jsonPath string) *ValidationError {
 
 // PositiveInteger validates that a value is at least 1.
 // Returns nil if valid, *ValidationError if invalid.
+// It delegates to TimeoutMinimum with min=1, then overrides Message and
+// Suggestion to use "positive integer" phrasing instead of the generic
+// "must be at least N" wording produced by TimeoutMinimum.
 func PositiveInteger(value int, fieldName, jsonPath string) *ValidationError {
 	logValidation.Printf("Validating positive integer: field=%s, value=%d, jsonPath=%s", fieldName, value, jsonPath)
-	if value < 1 {
-		return newValidationError(
-			fmt.Sprintf("Positive integer validation failed: %s=%d is not positive", fieldName, value),
-			fieldName,
-			fmt.Sprintf("%s must be a positive integer (>= 1), got %d", fieldName, value),
-			jsonPath,
-			fmt.Sprintf("Use a positive integer (>= 1) for %s", fieldName),
-		)
+	if err := TimeoutMinimum(value, 1, fieldName, jsonPath); err != nil {
+		logValidation.Printf("Positive integer validation failed: %s=%d is not positive", fieldName, value)
+		err.Message = fmt.Sprintf("%s must be a positive integer (>= 1), got %d", fieldName, value)
+		err.Suggestion = fmt.Sprintf("Use a positive integer (>= 1) for %s", fieldName)
+		return err
 	}
 	return nil
 }

internal/difc/sink_server_ids.go

@@ -6,7 +6,7 @@ import (
 	"sync"
 
 	"github.com/github/gh-aw-mcpg/internal/logger"
-	"github.com/github/gh-aw-mcpg/internal/strutil"
+	"github.com/github/gh-aw-mcpg/internal/util"
 )
 
 var logSink = logger.New("difc:sink_server_ids")
@@ -31,7 +31,7 @@ func SetSinkServerIDs(serverIDs []string) {
 		return
 	}
 
-	normalized := strutil.DeduplicateStrings(serverIDs, true)
+	normalized := util.DeduplicateStrings(serverIDs, true)
 	if len(normalized) < len(serverIDs) {
 		logSink.Printf("Removed %d duplicate or empty sink server IDs", len(serverIDs)-len(normalized))
 	}
@@ -82,7 +82,7 @@ func ParseSinkServerIDs(input string) ([]string, error) {
 		validated = append(validated, value)
 	}
 
-	result := strutil.DeduplicateStrings(validated, false)
+	result := util.DeduplicateStrings(validated, false)
 	logSink.Printf("Parsed %d unique DIFC sink server IDs: %v", len(result), result)
 	return result, nil
 }

internal/githubhttp/collaborator.go

@@ -9,7 +9,7 @@ import (
 	"github.com/github/gh-aw-mcpg/internal/httputil"
 
 	"github.com/github/gh-aw-mcpg/internal/logger"
-	"github.com/github/gh-aw-mcpg/internal/strutil"
+	"github.com/github/gh-aw-mcpg/internal/util"
 )
 
 var logCollab = logger.New("githubhttp:collaborator")
@@ -19,9 +19,9 @@ var logCollab = logger.New("githubhttp:collaborator")
 // It returns the (possibly partial) values even on error so that callers can
 // include them in diagnostic log messages.
 func ParseCollaboratorPermissionArgs(argsMap map[string]interface{}) (owner, repo, username string, err error) {
-	owner = strutil.GetStringFromMap(argsMap, "owner")
-	repo = strutil.GetStringFromMap(argsMap, "repo")
-	username = strutil.GetStringFromMap(argsMap, "username")
+	owner = util.GetStringFromMap(argsMap, "owner")
+	repo = util.GetStringFromMap(argsMap, "repo")
+	username = util.GetStringFromMap(argsMap, "username")
 	if owner == "" || repo == "" || username == "" {
 		logCollab.Printf("ParseCollaboratorPermissionArgs: missing required fields: owner=%q, repo=%q, username=%q", owner, repo, username)
 		err = fmt.Errorf("get_collaborator_permission: missing owner/repo/username")

internal/guard/wasm_payload.go

@@ -6,7 +6,7 @@ import (
 	"strings"
 
 	"github.com/github/gh-aw-mcpg/internal/config"
-	"github.com/github/gh-aw-mcpg/internal/strutil"
+	"github.com/github/gh-aw-mcpg/internal/util"
 )
 
 // normalizePolicyPayload coerces a policy value to a map[string]interface{}.
@@ -251,7 +251,7 @@ func BuildLabelAgentPayload(policy interface{}, trustedBots []string, trustedUse
 
 	if len(trustedBots) > 0 {
 		// trusted-bots is a top-level key in the label_agent payload.
-		bots := strutil.StringsToAny(trustedBots)
+		bots := util.StringsToAny(trustedBots)
 		payload["trusted-bots"] = bots
 		logWasm.Printf("BuildLabelAgentPayload: injected %d trusted-bots into payload", len(trustedBots))
 	}
@@ -260,7 +260,7 @@ func BuildLabelAgentPayload(policy interface{}, trustedBots []string, trustedUse
 		// trusted-users is injected inside the allow-only object.
 		// If allow-only is absent, the injection is skipped and buildStrictLabelAgentPayload
 		// will reject the payload when called with the missing allow-only key.
-		users := strutil.StringsToAny(trustedUsers)
+		users := util.StringsToAny(trustedUsers)
 		// Inject into allow-only object if present
 		if allowOnly, ok := payload["allow-only"].(map[string]interface{}); ok {
 			allowOnly["trusted-users"] = users

internal/logger/fallback_handlers_test.go

@@ -8,11 +8,11 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-func TestHandleFileLoggerError_UsesFallback(t *testing.T) {
+func TestFileLoggerFactory_OnError_UsesFallback(t *testing.T) {
 	var logger *FileLogger
 	var err error
 	_ = captureStdLog(t, func() {
-		logger, err = handleFileLoggerError(assert.AnError, "/tmp/logs", "test.log")
+		logger, err = fileLoggerFactory.onError(assert.AnError, "/tmp/logs", "test.log")
 	})
 	require.NoError(t, err)
 	require.NotNil(t, logger)
@@ -22,11 +22,11 @@ func TestHandleFileLoggerError_UsesFallback(t *testing.T) {
 	assert.Equal(t, os.Stderr, logger.logger.Writer())
 }
 
-func TestHandleMarkdownLoggerError_UsesFallback(t *testing.T) {
+func TestMarkdownLoggerFactory_OnError_UsesFallback(t *testing.T) {
 	var logger *MarkdownLogger
 	var err error
 	_ = captureStdLog(t, func() {
-		logger, err = handleMarkdownLoggerError(assert.AnError, "/tmp/logs", "test.md")
+		logger, err = markdownLoggerFactory.onError(assert.AnError, "/tmp/logs", "test.md")
 	})
 	require.NoError(t, err)
 	require.NotNil(t, logger)
@@ -35,11 +35,11 @@ func TestHandleMarkdownLoggerError_UsesFallback(t *testing.T) {
 	assert.False(t, logger.initialized)
 }
 
-func TestHandleToolsLoggerError_UsesFallback(t *testing.T) {
+func TestToolsLoggerFactory_OnError_UsesFallback(t *testing.T) {
 	var logger *ToolsLogger
 	var err error
 	_ = captureStdLog(t, func() {
-		logger, err = handleToolsLoggerError(assert.AnError, "/tmp/logs", "tools.json")
+		logger, err = toolsLoggerFactory.onError(assert.AnError, "/tmp/logs", "tools.json")
 	})
 	require.NoError(t, err)
 	require.NotNil(t, logger)

internal/logger/file_logger.go

@@ -23,34 +23,28 @@ var (
 	globalLoggerMu   sync.RWMutex
 )
 
-// setupFileLogger configures a FileLogger after the log file has been opened.
-func setupFileLogger(file *os.File, logDir, fileName string) (*FileLogger, error) {
-	fl := &FileLogger{
-		logDir:   logDir,
-		fileName: fileName,
-		logFile:  file,
-		logger:   log.New(file, "", 0),
-	}
-	log.Printf("Logging to file: %s", filepath.Join(logDir, fileName))
-	return fl, nil
-}
-
-// handleFileLoggerError falls back to stderr when the log file cannot be opened.
-// Stderr is used (not stdout) to avoid corrupting the stdout JSON channel that
-// callers use to receive the gateway configuration output.
-func handleFileLoggerError(err error, logDir, fileName string) (*FileLogger, error) {
-	return fallbackLoggerOnInitError(err, "Failed to initialize log file", "Falling back to stderr for logging", &FileLogger{
-		logDir:      logDir,
-		fileName:    fileName,
-		useFallback: true,
-		logger:      log.New(os.Stderr, "", 0),
-	})
-}
-
 // fileLoggerFactory bundles the setup and error-handler for FileLogger.
 var fileLoggerFactory = loggerFactory[*FileLogger]{
-	setup:   setupFileLogger,
-	onError: handleFileLoggerError,
+	setup: func(file *os.File, logDir, fileName string) (*FileLogger, error) {
+		fl := &FileLogger{
+			logDir:   logDir,
+			fileName: fileName,
+			logFile:  file,
+			logger:   log.New(file, "", 0),
+		}
+		log.Printf("Logging to file: %s", filepath.Join(logDir, fileName))
+		return fl, nil
+	},
+	onError: func(err error, logDir, fileName string) (*FileLogger, error) {
+		// Stderr is used (not stdout) to avoid corrupting the stdout JSON channel that
+		// callers use to receive the gateway configuration output.
+		return fallbackLoggerOnInitError(err, "Failed to initialize log file", "Falling back to stderr for logging", &FileLogger{
+			logDir:      logDir,
+			fileName:    fileName,
+			useFallback: true,
+			logger:      log.New(os.Stderr, "", 0),
+		})
+	},
 }
 
 // InitFileLogger initializes the global file logger