refactor: consolidate map utilities, deep clone, and logger constant

internal/logger/jsonl_logger.go

@@ -25,8 +25,7 @@ var (
 )
 
 const (
-	logTimestampLayout  = "2006-01-02T15:04:05.000Z07:00"
-	jsonTimestampLayout = logTimestampLayout
+	jsonTimestampLayout = "2006-01-02T15:04:05.000Z07:00"
 	rpcMessageSchemaV2  = "rpc-message/v2"
 	difcSchemaV2        = "difc-filtered/v2"
 )

internal/proxy/proxy_test.go

@@ -8,7 +8,7 @@ import (
 	"testing"
 
 	"github.com/github/gh-aw-mcpg/internal/difc"
-	"github.com/github/gh-aw-mcpg/internal/jsonutil"
+	"github.com/github/gh-aw-mcpg/internal/strutil"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -780,7 +780,7 @@ func TestDeepCloneJSON(t *testing.T) {
 		"a": []interface{}{float64(1), float64(2)},
 		"b": map[string]interface{}{"c": "d"},
 	}
-	cloned := jsonutil.DeepCloneJSON(original)
+	cloned := strutil.DeepCloneJSON(original)
 	// Mutate original
 	original["a"].([]interface{})[0] = float64(99)
 	original["b"].(map[string]interface{})["c"] = "mutated"

internal/proxy/response_transform.go

@@ -4,8 +4,8 @@ import (
 	"reflect"
 
 	"github.com/github/gh-aw-mcpg/internal/difc"
-	"github.com/github/gh-aw-mcpg/internal/jsonutil"
 	"github.com/github/gh-aw-mcpg/internal/logger"
+	"github.com/github/gh-aw-mcpg/internal/strutil"
 )
 
 var logTransform = logger.New("proxy:response_transform")
@@ -111,7 +111,7 @@ func rebuildGraphQLResponse(originalData interface{}, filtered *difc.FilteredCol
 	logTransform.Printf("rebuildGraphQLResponse: rebuilding with %d accessible items", filtered.GetAccessibleCount())
 
 	// Deep-clone the original data structure
-	cloned := jsonutil.DeepCloneJSON(original)
+	cloned := strutil.DeepCloneJSON(original)
 
 	// Build accessible items set
 	accessibleItems := make([]interface{}, 0, len(filtered.Accessible))

internal/proxy/response_transform_coverage_test.go

@@ -4,7 +4,7 @@ import (
 	"testing"
 
 	"github.com/github/gh-aw-mcpg/internal/difc"
-	"github.com/github/gh-aw-mcpg/internal/jsonutil"
+	"github.com/github/gh-aw-mcpg/internal/strutil"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -363,7 +363,7 @@ func TestDeepCloneJSON_Map(t *testing.T) {
 		"tags": []interface{}{"go", "test"},
 	}
 
-	cloned := jsonutil.DeepCloneJSON(original)
+	cloned := strutil.DeepCloneJSON(original)
 
 	clonedMap, ok := cloned.(map[string]interface{})
 	require.True(t, ok, "cloned value should be a map")
@@ -390,7 +390,7 @@ func TestDeepCloneJSON_NestedMapAndSlice(t *testing.T) {
 		},
 	}
 
-	cloned := jsonutil.DeepCloneJSON(original)
+	cloned := strutil.DeepCloneJSON(original)
 
 	// Mutate deeply nested value.
 	original["level1"].(map[string]interface{})["level2"].([]interface{})[0].(map[string]interface{})["x"] = float64(99)

internal/proxy/response_transform_test.go

@@ -4,7 +4,7 @@ import (
 	"testing"
 
 	"github.com/github/gh-aw-mcpg/internal/difc"
-	"github.com/github/gh-aw-mcpg/internal/jsonutil"
+	"github.com/github/gh-aw-mcpg/internal/strutil"
 	"github.com/stretchr/testify/assert"
 )
 
@@ -111,7 +111,7 @@ func TestDeepCloneJSON_Slice(t *testing.T) {
 		float64(42),
 	}
 
-	cloned := jsonutil.DeepCloneJSON(original)
+	cloned := strutil.DeepCloneJSON(original)
 
 	clonedSlice, ok := cloned.([]interface{})
 	assert.True(t, ok, "cloned value should be a []interface{}")
@@ -128,12 +128,12 @@ func TestDeepCloneJSON_Slice(t *testing.T) {
 // TestDeepCloneJSON_Primitive verifies that deepCloneJSON returns primitive values
 // (string, number, bool, nil) unchanged — they are immutable so no copy is needed.
 func TestDeepCloneJSON_Primitive(t *testing.T) {
-	assert.Equal(t, "hello", jsonutil.DeepCloneJSON("hello"))
-	assert.Equal(t, float64(3.14), jsonutil.DeepCloneJSON(float64(3.14)))
-	clonedBool, ok := jsonutil.DeepCloneJSON(true).(bool)
+	assert.Equal(t, "hello", strutil.DeepCloneJSON("hello"))
+	assert.Equal(t, float64(3.14), strutil.DeepCloneJSON(float64(3.14)))
+	clonedBool, ok := strutil.DeepCloneJSON(true).(bool)
 	assert.True(t, ok, "cloned bool should remain a bool")
 	assert.True(t, clonedBool)
-	assert.Nil(t, jsonutil.DeepCloneJSON(nil))
+	assert.Nil(t, strutil.DeepCloneJSON(nil))
 }
 
 // TestUnwrapSingleObject_NilFilteredData verifies that unwrapSingleObject handles

internal/server/difc_log.go

@@ -7,6 +7,7 @@ import (
 
 	"github.com/github/gh-aw-mcpg/internal/difc"
 	"github.com/github/gh-aw-mcpg/internal/logger"
+	"github.com/github/gh-aw-mcpg/internal/strutil"
 )
 
 var logDifcLog = logger.New("server:difc_log")
@@ -48,32 +49,17 @@ func buildFilteredItemLogEntry(serverID, toolName string, detail difc.FilteredIt
 	// Extract identifying metadata from the raw item data.
 	// Data is interface{} from JSON parsing — typically map[string]interface{}.
 	if m, ok := detail.Item.Data.(map[string]interface{}); ok {
-		entry.AuthorAssociation = getFilteredItemStringField(m, "author_association", "authorAssociation")
+		entry.AuthorAssociation = strutil.GetStringFromMap(m, "author_association", "authorAssociation")
 		entry.AuthorLogin = extractAuthorLogin(m)
-		entry.HTMLURL = getFilteredItemStringField(m, "html_url", "htmlUrl")
+		entry.HTMLURL = strutil.GetStringFromMap(m, "html_url", "htmlUrl")
 		entry.Number = extractNumberField(m)
-		entry.SHA = getFilteredItemStringField(m, "sha")
+		entry.SHA = strutil.GetStringFromMap(m, "sha")
 		logDifcLog.Printf("Filtered item metadata: author=%s, number=%s, url=%s", entry.AuthorLogin, entry.Number, entry.HTMLURL)
 	}
 
 	return entry
 }
 
-// getFilteredItemStringField returns the first non-empty string value from the map
-// matching any of the given field names.
-// NOTE: This helper is intentionally generic; if similar untyped JSON map extraction
-// patterns appear elsewhere, consider moving it to a shared utility package.
-func getFilteredItemStringField(m map[string]interface{}, fields ...string) string {
-	for _, f := range fields {
-		if v, ok := m[f]; ok {
-			if s, ok := v.(string); ok && s != "" {
-				return s
-			}
-		}
-	}
-	return ""
-}
-
 // extractAuthorLogin extracts the author login from nested user/author objects.
 func extractAuthorLogin(m map[string]interface{}) string {
 	if user, ok := m["user"].(map[string]interface{}); ok {

internal/server/difc_log_helpers_test.go

@@ -7,103 +7,6 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
-// TestGetFilteredItemStringField exercises all code paths of the
-// getFilteredItemStringField helper.
-func TestGetFilteredItemStringField(t *testing.T) {
-	tests := []struct {
-		name   string
-		m      map[string]interface{}
-		fields []string
-		want   string
-	}{
-		{
-			name:   "empty map returns empty string",
-			m:      map[string]interface{}{},
-			fields: []string{"title"},
-			want:   "",
-		},
-		{
-			name:   "single field found returns value",
-			m:      map[string]interface{}{"title": "My Issue"},
-			fields: []string{"title"},
-			want:   "My Issue",
-		},
-		{
-			name:   "single field present but empty string is skipped",
-			m:      map[string]interface{}{"title": ""},
-			fields: []string{"title"},
-			want:   "",
-		},
-		{
-			name:   "first field missing second field found",
-			m:      map[string]interface{}{"htmlUrl": "https://example.com"},
-			fields: []string{"html_url", "htmlUrl"},
-			want:   "https://example.com",
-		},
-		{
-			name:   "first field found returns first match",
-			m:      map[string]interface{}{"html_url": "https://first.com", "htmlUrl": "https://second.com"},
-			fields: []string{"html_url", "htmlUrl"},
-			want:   "https://first.com",
-		},
-		{
-			name:   "all fields missing returns empty string",
-			m:      map[string]interface{}{"other": "value"},
-			fields: []string{"html_url", "htmlUrl"},
-			want:   "",
-		},
-		{
-			name:   "field present with non-string value is skipped",
-			m:      map[string]interface{}{"number": 42},
-			fields: []string{"number"},
-			want:   "",
-		},
-		{
-			name:   "field present with bool value is skipped",
-			m:      map[string]interface{}{"active": true},
-			fields: []string{"active"},
-			want:   "",
-		},
-		{
-			name:   "field present with nil value is skipped",
-			m:      map[string]interface{}{"login": nil},
-			fields: []string{"login"},
-			want:   "",
-		},
-		{
-			name:   "first field non-string second field string returns second",
-			m:      map[string]interface{}{"author_association": 99, "authorAssociation": "CONTRIBUTOR"},
-			fields: []string{"author_association", "authorAssociation"},
-			want:   "CONTRIBUTOR",
-		},
-		{
-			name:   "no fields argument returns empty string",
-			m:      map[string]interface{}{"title": "value"},
-			fields: []string{},
-			want:   "",
-		},
-		{
-			name:   "nil map returns empty string",
-			m:      nil,
-			fields: []string{"field"},
-			want:   "",
-		},
-		{
-			name:   "sha field extraction",
-			m:      map[string]interface{}{"sha": "abc123def456"},
-			fields: []string{"sha"},
-			want:   "abc123def456",
-		},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			got := getFilteredItemStringField(tt.m, tt.fields...)
-			assert.Equal(t, tt.want, got)
-		})
-	}
-}
-
 // TestExtractAuthorLogin exercises all code paths of the extractAuthorLogin helper.
 func TestExtractAuthorLogin(t *testing.T) {
 	tests := []struct {

internal/jsonutil/clone.go → internal/strutil/deepclone.go

@@ -1,5 +1,4 @@
-// Package jsonutil provides utilities for working with JSON-compatible values.
-package jsonutil
+package strutil
 
 // DeepCloneJSON creates a deep copy of a JSON-compatible value.
 func DeepCloneJSON(v interface{}) interface{} {

internal/strutil/maputil.go

@@ -0,0 +1,26 @@
+package strutil
+
+// GetStringFromMap returns the first non-empty string value found for any of
+// the given keys in m.  For each key, the value must be present, typed as
+// string, and non-empty to be returned.  Returns an empty string when no
+// matching non-empty string value is found, when the map is nil, or when no
+// keys are provided.
+//
+// With a single key the behaviour is equivalent to `v, _ := m[key].(string)`:
+//
+//	GetStringFromMap(m, "owner")
+//
+// With multiple keys the function returns the first non-empty match, which is
+// useful for maps that may use either snake_case or camelCase field names:
+//
+//	GetStringFromMap(m, "html_url", "htmlUrl")
+func GetStringFromMap(m map[string]interface{}, keys ...string) string {
+	for _, k := range keys {
+		if v, ok := m[k]; ok {
+			if s, ok := v.(string); ok && s != "" {
+				return s
+			}
+		}
+	}
+	return ""
+}

internal/strutil/truncate.go

@@ -54,10 +54,3 @@ func TruncateRunes(s string, maxRunes int) string {
 	}
 	return string(r[:maxRunes])
 }
-
-// GetStringFromMap returns the string value for key when it is present and typed as string.
-// It returns an empty string for missing keys, nil maps, and non-string values.
-func GetStringFromMap(m map[string]interface{}, key string) string {
-	v, _ := m[key].(string)
-	return v
-}

internal/strutil/util_test.go

@@ -32,4 +32,46 @@ func TestGetStringFromMap(t *testing.T) {
 		var m map[string]interface{}
 		assert.Equal(t, "", GetStringFromMap(m, "owner"))
 	})
+
+	t.Run("returns empty string when value is empty string", func(t *testing.T) {
+		t.Parallel()
+		m := map[string]interface{}{"owner": ""}
+		assert.Equal(t, "", GetStringFromMap(m, "owner"))
+	})
+
+	t.Run("variadic: returns first non-empty match", func(t *testing.T) {
+		t.Parallel()
+		m := map[string]interface{}{"htmlUrl": "https://example.com"}
+		assert.Equal(t, "https://example.com", GetStringFromMap(m, "html_url", "htmlUrl"))
+	})
+
+	t.Run("variadic: first key takes priority when both present", func(t *testing.T) {
+		t.Parallel()
+		m := map[string]interface{}{"html_url": "https://first.com", "htmlUrl": "https://second.com"}
+		assert.Equal(t, "https://first.com", GetStringFromMap(m, "html_url", "htmlUrl"))
+	})
+
+	t.Run("variadic: skips empty first key and returns second", func(t *testing.T) {
+		t.Parallel()
+		m := map[string]interface{}{"html_url": "", "htmlUrl": "https://second.com"}
+		assert.Equal(t, "https://second.com", GetStringFromMap(m, "html_url", "htmlUrl"))
+	})
+
+	t.Run("variadic: all keys missing returns empty string", func(t *testing.T) {
+		t.Parallel()
+		m := map[string]interface{}{"other": "value"}
+		assert.Equal(t, "", GetStringFromMap(m, "html_url", "htmlUrl"))
+	})
+
+	t.Run("variadic: skips non-string first key and returns second", func(t *testing.T) {
+		t.Parallel()
+		m := map[string]interface{}{"html_url": 99, "htmlUrl": "https://second.com"}
+		assert.Equal(t, "https://second.com", GetStringFromMap(m, "html_url", "htmlUrl"))
+	})
+
+	t.Run("no keys returns empty string", func(t *testing.T) {
+		t.Parallel()
+		m := map[string]interface{}{"owner": "octo"}
+		assert.Equal(t, "", GetStringFromMap(m))
+	})
 }