From 7476365c511c592a6d961cb273f5a9576c6f7c85 Mon Sep 17 00:00:00 2001
From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com>
Date: Tue, 26 May 2026 22:10:54 +0000
Subject: [PATCH 1/2] Initial plan


From e4d12c9f6178d1d6480126a7cf197059d31a1902 Mon Sep 17 00:00:00 2001
From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com>
Date: Tue, 26 May 2026 22:16:31 +0000
Subject: [PATCH 2/2] Refactor misplaced config and tracing helpers

---
 internal/cmd/flags_difc.go          |  15 +++
 internal/cmd/wasm_cache.go          |  15 ---
 internal/config/config_tracing.go   |  40 ------
 internal/config/expand.go           |  40 ++++++
 internal/tracing/config_resolver.go | 196 ++++++++++++++++++++++++++++
 internal/tracing/provider.go        | 187 --------------------------
 6 files changed, 251 insertions(+), 242 deletions(-)
 create mode 100644 internal/tracing/config_resolver.go

diff --git a/internal/cmd/flags_difc.go b/internal/cmd/flags_difc.go
index f21cfcfa9..27a9f1612 100644
--- a/internal/cmd/flags_difc.go
+++ b/internal/cmd/flags_difc.go
@@ -25,6 +25,9 @@ var (
 	allowOnlyMinInt   string
 )
 
+// containerGuardWasmPath is the baked-in guard path in the container image.
+const containerGuardWasmPath = "/guards/github/00-github-guard.wasm"
+
 func init() {
 	RegisterFlag(func(cmd *cobra.Command) {
 		cmd.Flags().StringVar(&difcMode, "guards-mode", getDefaultDIFCMode(), "Guards enforcement mode: strict (deny violations), filter (remove denied tools), or propagate (auto-adjust agent labels on reads)")
@@ -37,6 +40,18 @@ func init() {
 	})
 }
 
+// detectGuardWasm returns the baked-in container guard path if it exists,
+// or empty string if not found (requiring the user to specify --guard-wasm).
+func detectGuardWasm() string {
+	debugLog.Printf("Checking for baked-in guard at %s", containerGuardWasmPath)
+	if _, err := os.Stat(containerGuardWasmPath); err == nil {
+		debugLog.Printf("Auto-detected baked-in guard: %s", containerGuardWasmPath)
+		return containerGuardWasmPath
+	}
+	debugLog.Print("Baked-in guard not found, --guard-wasm flag required")
+	return ""
+}
+
 func resolveGuardPolicyOverride(cmd *cobra.Command) (*config.GuardPolicy, string, error) {
 	cliGuardPolicyChanged := cmd.Flags().Changed("guard-policy-json")
 	cliChanged := cliGuardPolicyChanged ||
diff --git a/internal/cmd/wasm_cache.go b/internal/cmd/wasm_cache.go
index eedad4641..be89b4dd5 100644
--- a/internal/cmd/wasm_cache.go
+++ b/internal/cmd/wasm_cache.go
@@ -12,21 +12,6 @@ import (
 	"github.com/github/gh-aw-mcpg/internal/guard"
 )
 
-// containerGuardWasmPath is the baked-in guard path in the container image.
-const containerGuardWasmPath = "/guards/github/00-github-guard.wasm"
-
-// detectGuardWasm returns the baked-in container guard path if it exists,
-// or empty string if not found (requiring the user to specify --guard-wasm).
-func detectGuardWasm() string {
-	debugLog.Printf("Checking for baked-in guard at %s", containerGuardWasmPath)
-	if _, err := os.Stat(containerGuardWasmPath); err == nil {
-		debugLog.Printf("Auto-detected baked-in guard: %s", containerGuardWasmPath)
-		return containerGuardWasmPath
-	}
-	debugLog.Print("Baked-in guard not found, --guard-wasm flag required")
-	return ""
-}
-
 func defaultWasmCacheDir(logDir string) string {
 	if logDir == "" {
 		return config.DefaultWasmCacheDirName
diff --git a/internal/config/config_tracing.go b/internal/config/config_tracing.go
index 3e5fd6cbb..9285de708 100644
--- a/internal/config/config_tracing.go
+++ b/internal/config/config_tracing.go
@@ -102,43 +102,3 @@ func init() {
 		}
 	})
 }
-
-// expandTracingVariables expands ${VAR} expressions in TracingConfig fields.
-// This is called for TOML-loaded configs before validation, mirroring the
-// stdin JSON path where ExpandRawJSONVariables handles expansion.
-func expandTracingVariables(cfg *TracingConfig) error {
-	if cfg == nil {
-		return nil
-	}
-
-	logValidation.Printf("Expanding tracing config variables: hasEndpoint=%v, hasTraceID=%v, hasSpanID=%v, hasHeaders=%v",
-		cfg.Endpoint != "", cfg.TraceID != "", cfg.SpanID != "", cfg.Headers != "")
-
-	fields := []struct {
-		name     string
-		jsonPath string
-		value    *string
-	}{
-		{name: "endpoint", jsonPath: "gateway.opentelemetry.endpoint", value: &cfg.Endpoint},
-		{name: "traceId", jsonPath: "gateway.opentelemetry.traceId", value: &cfg.TraceID},
-		{name: "spanId", jsonPath: "gateway.opentelemetry.spanId", value: &cfg.SpanID},
-		{name: "headers", jsonPath: "gateway.opentelemetry.headers", value: &cfg.Headers},
-	}
-
-	for _, field := range fields {
-		if *field.value == "" {
-			continue
-		}
-
-		expanded, err := expandVariables(*field.value, field.jsonPath)
-		if err != nil {
-			return err
-		}
-
-		logValidation.Printf("Expanded tracing %s variable", field.name)
-		*field.value = expanded
-	}
-
-	logValidation.Print("Tracing config variable expansion completed")
-	return nil
-}
diff --git a/internal/config/expand.go b/internal/config/expand.go
index 3323845ad..53801c672 100644
--- a/internal/config/expand.go
+++ b/internal/config/expand.go
@@ -88,3 +88,43 @@ func expandEnvVariables(env map[string]string, serverName string) (map[string]st
 	logValidation.Printf("Env variable expansion completed for server: %s", serverName)
 	return result, nil
 }
+
+// expandTracingVariables expands ${VAR} expressions in TracingConfig fields.
+// This is called for TOML-loaded configs before validation, mirroring the
+// stdin JSON path where ExpandRawJSONVariables handles expansion.
+func expandTracingVariables(cfg *TracingConfig) error {
+	if cfg == nil {
+		return nil
+	}
+
+	logValidation.Printf("Expanding tracing config variables: hasEndpoint=%v, hasTraceID=%v, hasSpanID=%v, hasHeaders=%v",
+		cfg.Endpoint != "", cfg.TraceID != "", cfg.SpanID != "", cfg.Headers != "")
+
+	fields := []struct {
+		name     string
+		jsonPath string
+		value    *string
+	}{
+		{name: "endpoint", jsonPath: "gateway.opentelemetry.endpoint", value: &cfg.Endpoint},
+		{name: "traceId", jsonPath: "gateway.opentelemetry.traceId", value: &cfg.TraceID},
+		{name: "spanId", jsonPath: "gateway.opentelemetry.spanId", value: &cfg.SpanID},
+		{name: "headers", jsonPath: "gateway.opentelemetry.headers", value: &cfg.Headers},
+	}
+
+	for _, field := range fields {
+		if *field.value == "" {
+			continue
+		}
+
+		expanded, err := expandVariables(*field.value, field.jsonPath)
+		if err != nil {
+			return err
+		}
+
+		logValidation.Printf("Expanded tracing %s variable", field.name)
+		*field.value = expanded
+	}
+
+	logValidation.Print("Tracing config variable expansion completed")
+	return nil
+}
diff --git a/internal/tracing/config_resolver.go b/internal/tracing/config_resolver.go
new file mode 100644
index 000000000..d8cc59c40
--- /dev/null
+++ b/internal/tracing/config_resolver.go
@@ -0,0 +1,196 @@
+package tracing
+
+import (
+	"context"
+	"encoding/hex"
+	"net/url"
+	"os"
+	"strings"
+
+	"go.opentelemetry.io/otel/trace"
+
+	"github.com/github/gh-aw-mcpg/internal/config"
+)
+
+// defaultSignalPath is the OTLP traces signal path per the OpenTelemetry spec.
+const defaultSignalPath = "/v1/traces"
+
+// resolveEndpoint returns the OTLP endpoint from config.
+// CLI flags set the config value using env vars as defaults, so config already
+// reflects the correct precedence: CLI flag > env var > config file.
+//
+// Per the OpenTelemetry specification, OTEL_EXPORTER_OTLP_ENDPOINT is a base URL
+// and SDKs must append the signal path (/v1/traces for traces). Since we use
+// WithEndpointURL (which takes the URL as-is), we append the signal path here
+// when it is not already present. The path defaults to /v1/traces but can be
+// overridden via TracingConfig.SignalPath.
+func resolveEndpoint(cfg *config.TracingConfig) string {
+	if cfg == nil || cfg.Endpoint == "" {
+		return ""
+	}
+	endpoint := cfg.Endpoint
+	signalPath := cfg.SignalPath
+	if signalPath == "" {
+		signalPath = defaultSignalPath
+	}
+
+	u, err := url.Parse(endpoint)
+	if err != nil {
+		// If unparseable, fall back to string append for best-effort.
+		// Normalize trailing slashes before the suffix check to avoid
+		// duplicating the signal path when input already ends with it.
+		normalized := strings.TrimRight(endpoint, "/")
+		if !strings.HasSuffix(normalized, signalPath) {
+			normalized += signalPath
+		}
+		return normalized
+	}
+
+	// Normalize path and check whether signal path is already the suffix
+	normalizedPath := strings.TrimRight(u.Path, "/")
+	if !strings.HasSuffix(normalizedPath, signalPath) {
+		u.Path = normalizedPath + signalPath
+	} else {
+		u.Path = normalizedPath
+	}
+	return u.String()
+}
+
+// resolveServiceName returns the service name from config.
+func resolveServiceName(cfg *config.TracingConfig) string {
+	if cfg != nil && cfg.ServiceName != "" {
+		return cfg.ServiceName
+	}
+	return config.DefaultTracingServiceName
+}
+
+// resolveSampleRate returns the sample rate from config (defaults to 1.0).
+// Valid configured values are in the range [0.0, 1.0], where 0.0 disables sampling.
+func resolveSampleRate(cfg *config.TracingConfig) float64 {
+	rate := cfg.GetSampleRate()
+
+	if rate >= 0.0 && rate <= 1.0 {
+		return rate
+	}
+
+	logTracing.Printf("Warning: invalid tracing sample rate %.4f; using default %.2f", rate, config.DefaultTracingSampleRate)
+	return config.DefaultTracingSampleRate
+}
+
+// parseOTLPHeaders parses a comma-separated "key=value" string into a map.
+// Empty pairs, pairs without "=", and pairs with an empty key are logged as
+// warnings and skipped to avoid invalid HTTP header field names.
+// Leading/trailing whitespace around keys and values is trimmed.
+func parseOTLPHeaders(raw string) map[string]string {
+	return parseOTLPHeadersWithDecoder(raw, false)
+}
+
+func parseOTLPHeadersWithDecoder(raw string, decodeValues bool) map[string]string {
+	headers := make(map[string]string)
+	for _, pair := range strings.Split(raw, ",") {
+		trimmed := strings.TrimSpace(pair)
+		if trimmed == "" {
+			continue
+		}
+		k, v, ok := strings.Cut(trimmed, "=")
+		if !ok {
+			logTracing.Printf("Warning: skipping malformed OTLP header pair (missing '=')")
+			continue
+		}
+		key := strings.TrimSpace(k)
+		if key == "" {
+			logTracing.Printf("Warning: skipping OTLP header pair with empty key")
+			continue
+		}
+		value := strings.TrimSpace(v)
+		if decodeValues {
+			decoded, err := url.PathUnescape(value)
+			if err != nil {
+				logTracing.Printf("Warning: invalid percent-encoding in OTLP header value for key %q; using raw value", key)
+			} else {
+				value = decoded
+			}
+		}
+		headers[key] = value
+	}
+	return headers
+}
+
+// resolveHeaders parses the configured OTLP export headers string (or returns nil).
+// When no headers are configured via config, it falls back to the standard
+// OTEL_EXPORTER_OTLP_HEADERS environment variable (W3C Baggage format:
+// "key1=value1,key2=value2") per the OTel OTLP Exporter specification.
+func resolveHeaders(cfg *config.TracingConfig) map[string]string {
+	raw := ""
+	if cfg != nil {
+		raw = cfg.Headers
+	}
+	if raw == "" {
+		raw = os.Getenv("OTEL_EXPORTER_OTLP_HEADERS")
+		if raw != "" {
+			logTracing.Printf("Using OTEL_EXPORTER_OTLP_HEADERS env var for OTLP export headers")
+		}
+	}
+	if raw == "" {
+		return nil
+	}
+	if cfg == nil || cfg.Headers == "" {
+		return parseOTLPHeadersWithDecoder(raw, true)
+	}
+	return parseOTLPHeaders(raw)
+}
+
+// resolveParentContext builds a context carrying the W3C remote parent span context
+// from the configured traceId and spanId (spec §4.1.3.6).
+// If traceId is absent, or either ID is malformed, the original context is returned unchanged.
+// A missing spanId is replaced with a random span ID so the traceparent is still valid.
+func resolveParentContext(ctx context.Context, cfg *config.TracingConfig) context.Context {
+	if cfg == nil || cfg.TraceID == "" {
+		return ctx
+	}
+
+	traceIDBytes, err := hex.DecodeString(cfg.TraceID)
+	if err != nil || len(traceIDBytes) != 16 {
+		logTracing.Printf("Warning: invalid traceId '%s'; skipping W3C parent context", cfg.TraceID)
+		return ctx
+	}
+	var traceID trace.TraceID
+	copy(traceID[:], traceIDBytes)
+
+	var spanID trace.SpanID
+	if cfg.SpanID != "" {
+		spanIDBytes, err := hex.DecodeString(cfg.SpanID)
+		if err != nil || len(spanIDBytes) != 8 {
+			logTracing.Printf("Warning: invalid spanId '%s'; generating a random span ID", cfg.SpanID)
+			// Fall through to generate a random span ID below
+		} else {
+			copy(spanID[:], spanIDBytes)
+		}
+	}
+
+	// When spanId is all-zeros (absent or invalid), generate a random span ID.
+	// A valid SpanContext requires a non-zero SpanID (W3C Trace Context spec).
+	// T-OTEL-008: when only traceId is provided, a random spanId is generated.
+	if spanID == (trace.SpanID{}) {
+		generatedID, genErr := generateRandomSpanID()
+		if genErr != nil {
+			logTracing.Printf("Warning: failed to generate random span ID: %v; skipping W3C parent context", genErr)
+			return ctx
+		}
+		spanID = generatedID
+		logTracing.Printf("Generated random spanId for W3C parent context")
+	}
+
+	sc := trace.NewSpanContext(trace.SpanContextConfig{
+		TraceID:    traceID,
+		SpanID:     spanID,
+		TraceFlags: trace.FlagsSampled,
+		Remote:     true,
+	})
+	if !sc.IsValid() {
+		logTracing.Printf("Warning: constructed parent SpanContext is not valid; skipping W3C parent context")
+		return ctx
+	}
+	logTracing.Printf("W3C parent context resolved: traceId=%s, spanId=%s", traceID, spanID)
+	return trace.ContextWithRemoteSpanContext(ctx, sc)
+}
diff --git a/internal/tracing/provider.go b/internal/tracing/provider.go
index 5a0dd978d..91f92de70 100644
--- a/internal/tracing/provider.go
+++ b/internal/tracing/provider.go
@@ -20,11 +20,7 @@ package tracing
 import (
 	"context"
 	"crypto/rand"
-	"encoding/hex"
 	"fmt"
-	"net/url"
-	"os"
-	"strings"
 	"time"
 
 	"go.opentelemetry.io/otel"
@@ -66,189 +62,6 @@ func (p *Provider) Shutdown(ctx context.Context) error {
 	return nil
 }
 
-// defaultSignalPath is the OTLP traces signal path per the OpenTelemetry spec.
-const defaultSignalPath = "/v1/traces"
-
-// resolveEndpoint returns the OTLP endpoint from config.
-// CLI flags set the config value using env vars as defaults, so config already
-// reflects the correct precedence: CLI flag > env var > config file.
-//
-// Per the OpenTelemetry specification, OTEL_EXPORTER_OTLP_ENDPOINT is a base URL
-// and SDKs must append the signal path (/v1/traces for traces). Since we use
-// WithEndpointURL (which takes the URL as-is), we append the signal path here
-// when it is not already present. The path defaults to /v1/traces but can be
-// overridden via TracingConfig.SignalPath.
-func resolveEndpoint(cfg *config.TracingConfig) string {
-	if cfg == nil || cfg.Endpoint == "" {
-		return ""
-	}
-	endpoint := cfg.Endpoint
-	signalPath := cfg.SignalPath
-	if signalPath == "" {
-		signalPath = defaultSignalPath
-	}
-
-	u, err := url.Parse(endpoint)
-	if err != nil {
-		// If unparseable, fall back to string append for best-effort.
-		// Normalize trailing slashes before the suffix check to avoid
-		// duplicating the signal path when input already ends with it.
-		normalized := strings.TrimRight(endpoint, "/")
-		if !strings.HasSuffix(normalized, signalPath) {
-			normalized += signalPath
-		}
-		return normalized
-	}
-
-	// Normalize path and check whether signal path is already the suffix
-	normalizedPath := strings.TrimRight(u.Path, "/")
-	if !strings.HasSuffix(normalizedPath, signalPath) {
-		u.Path = normalizedPath + signalPath
-	} else {
-		u.Path = normalizedPath
-	}
-	return u.String()
-}
-
-// resolveServiceName returns the service name from config.
-func resolveServiceName(cfg *config.TracingConfig) string {
-	if cfg != nil && cfg.ServiceName != "" {
-		return cfg.ServiceName
-	}
-	return config.DefaultTracingServiceName
-}
-
-// resolveSampleRate returns the sample rate from config (defaults to 1.0).
-// Valid configured values are in the range [0.0, 1.0], where 0.0 disables sampling.
-func resolveSampleRate(cfg *config.TracingConfig) float64 {
-	rate := cfg.GetSampleRate()
-
-	if rate >= 0.0 && rate <= 1.0 {
-		return rate
-	}
-
-	logTracing.Printf("Warning: invalid tracing sample rate %.4f; using default %.2f", rate, config.DefaultTracingSampleRate)
-	return config.DefaultTracingSampleRate
-}
-
-// parseOTLPHeaders parses a comma-separated "key=value" string into a map.
-// Empty pairs, pairs without "=", and pairs with an empty key are logged as
-// warnings and skipped to avoid invalid HTTP header field names.
-// Leading/trailing whitespace around keys and values is trimmed.
-func parseOTLPHeaders(raw string) map[string]string {
-	return parseOTLPHeadersWithDecoder(raw, false)
-}
-
-func parseOTLPHeadersWithDecoder(raw string, decodeValues bool) map[string]string {
-	headers := make(map[string]string)
-	for _, pair := range strings.Split(raw, ",") {
-		trimmed := strings.TrimSpace(pair)
-		if trimmed == "" {
-			continue
-		}
-		k, v, ok := strings.Cut(trimmed, "=")
-		if !ok {
-			logTracing.Printf("Warning: skipping malformed OTLP header pair (missing '=')")
-			continue
-		}
-		key := strings.TrimSpace(k)
-		if key == "" {
-			logTracing.Printf("Warning: skipping OTLP header pair with empty key")
-			continue
-		}
-		value := strings.TrimSpace(v)
-		if decodeValues {
-			decoded, err := url.PathUnescape(value)
-			if err != nil {
-				logTracing.Printf("Warning: invalid percent-encoding in OTLP header value for key %q; using raw value", key)
-			} else {
-				value = decoded
-			}
-		}
-		headers[key] = value
-	}
-	return headers
-}
-
-// resolveHeaders parses the configured OTLP export headers string (or returns nil).
-// When no headers are configured via config, it falls back to the standard
-// OTEL_EXPORTER_OTLP_HEADERS environment variable (W3C Baggage format:
-// "key1=value1,key2=value2") per the OTel OTLP Exporter specification.
-func resolveHeaders(cfg *config.TracingConfig) map[string]string {
-	raw := ""
-	if cfg != nil {
-		raw = cfg.Headers
-	}
-	if raw == "" {
-		raw = os.Getenv("OTEL_EXPORTER_OTLP_HEADERS")
-		if raw != "" {
-			logTracing.Printf("Using OTEL_EXPORTER_OTLP_HEADERS env var for OTLP export headers")
-		}
-	}
-	if raw == "" {
-		return nil
-	}
-	if cfg == nil || cfg.Headers == "" {
-		return parseOTLPHeadersWithDecoder(raw, true)
-	}
-	return parseOTLPHeaders(raw)
-}
-
-// resolveParentContext builds a context carrying the W3C remote parent span context
-// from the configured traceId and spanId (spec §4.1.3.6).
-// If traceId is absent, or either ID is malformed, the original context is returned unchanged.
-// A missing spanId is replaced with a random span ID so the traceparent is still valid.
-func resolveParentContext(ctx context.Context, cfg *config.TracingConfig) context.Context {
-	if cfg == nil || cfg.TraceID == "" {
-		return ctx
-	}
-
-	traceIDBytes, err := hex.DecodeString(cfg.TraceID)
-	if err != nil || len(traceIDBytes) != 16 {
-		logTracing.Printf("Warning: invalid traceId '%s'; skipping W3C parent context", cfg.TraceID)
-		return ctx
-	}
-	var traceID trace.TraceID
-	copy(traceID[:], traceIDBytes)
-
-	var spanID trace.SpanID
-	if cfg.SpanID != "" {
-		spanIDBytes, err := hex.DecodeString(cfg.SpanID)
-		if err != nil || len(spanIDBytes) != 8 {
-			logTracing.Printf("Warning: invalid spanId '%s'; generating a random span ID", cfg.SpanID)
-			// Fall through to generate a random span ID below
-		} else {
-			copy(spanID[:], spanIDBytes)
-		}
-	}
-
-	// When spanId is all-zeros (absent or invalid), generate a random span ID.
-	// A valid SpanContext requires a non-zero SpanID (W3C Trace Context spec).
-	// T-OTEL-008: when only traceId is provided, a random spanId is generated.
-	if spanID == (trace.SpanID{}) {
-		generatedID, genErr := generateRandomSpanID()
-		if genErr != nil {
-			logTracing.Printf("Warning: failed to generate random span ID: %v; skipping W3C parent context", genErr)
-			return ctx
-		}
-		spanID = generatedID
-		logTracing.Printf("Generated random spanId for W3C parent context")
-	}
-
-	sc := trace.NewSpanContext(trace.SpanContextConfig{
-		TraceID:    traceID,
-		SpanID:     spanID,
-		TraceFlags: trace.FlagsSampled,
-		Remote:     true,
-	})
-	if !sc.IsValid() {
-		logTracing.Printf("Warning: constructed parent SpanContext is not valid; skipping W3C parent context")
-		return ctx
-	}
-	logTracing.Printf("W3C parent context resolved: traceId=%s, spanId=%s", traceID, spanID)
-	return trace.ContextWithRemoteSpanContext(ctx, sc)
-}
-
 // generateRandomSpanID creates a cryptographically random 8-byte span ID.
 func generateRandomSpanID() (trace.SpanID, error) {
 	var id trace.SpanID
