Address PR feedback - simplify implementation and documentation

- Reverted function rename: NewToolResultResource now takes flag parameter directly
- Fixed duplicate flag checks: check once and reuse variable
- Fixed comments: "base64 data" → "raw binary data", "annotations" → "metadata"
- Simplified Antigravity docs: removed "soon support" and update references
- Simplified server-configuration.md: removed remote server mentions and redundant notes

Co-authored-by: SamMorrowDrums <4811358+SamMorrowDrums@users.noreply.github.com>

Author: Copilot

docs/installation-guides/install-antigravity.md

@@ -104,11 +104,7 @@ If you prefer running the server locally with Docker:
 
 ### File contents not displaying properly
 
-If you experience issues with the `get_file_contents` tool not displaying file contents correctly, this may be due to limited embedded resource support in some versions of Antigravity.
-
-**Solution:** The remote GitHub MCP Server will soon support the `MCP_DISABLE_EMBEDDED_RESOURCES` feature flag to improve compatibility. This flag makes file contents return as standard MCP content types that work better across different clients.
-
-For updates on this feature, see the [Server Configuration Guide](../server-configuration.md#feature-flags).
+If you experience issues with the `get_file_contents` tool not displaying file contents correctly, this may be due to limited embedded resource support in some versions of Antigravity. This is a known limitation that affects how file contents are displayed.
 
 ### "Error: serverUrl or command must be specified"
 

docs/server-configuration.md

@@ -351,43 +351,13 @@ The GitHub MCP Server supports runtime feature flags that can modify tool behavi
 
 ### MCP_DISABLE_EMBEDDED_RESOURCES
 
-**Purpose:** Improves compatibility with MCP clients that don't fully support embedded resources.
-
 When enabled, the `get_file_contents` tool returns file content as standard MCP content types instead of embedded resources:
 - **Text files**: Returned as `TextContent` with MIME type in metadata
-- **Binary files**: Returned as `ImageContent` with base64-encoded data
-
-**When to use:** Enable this flag if your MCP client has issues displaying or accessing file contents retrieved via `get_file_contents`.
+- **Binary files**: Returned as `ImageContent` with raw binary data
 
 **Configuration:**
 
-<table>
-<tr><th>Remote Server</th><th>Local Server</th></tr>
-<tr valign="top">
-<td>
-
-**Coming soon:** Remote server support for this flag will be available in the next release.
-
-</td>
-<td>
-
-Feature flags are checked at runtime via the feature flag checker. Configuration method depends on your deployment:
-
-**For custom integrations:**
-```go
-featureChecker := func(ctx context.Context, flagName string) (bool, error) {
-    if flagName == "MCP_DISABLE_EMBEDDED_RESOURCES" {
-        return true, nil // Enable the flag
-    }
-    return false, nil
-}
-```
-
-</td>
-</tr>
-</table>
-
-> **Note:** This feature flag does not affect other tools, only `get_file_contents`. The default behavior (embedded resources) is maintained when the flag is not enabled, ensuring backward compatibility.
+Feature flags are checked at runtime via the feature flag checker passed to `BaseDeps`. You can configure this through environment variables or command-line arguments depending on your deployment setup.
 
 ---
 

pkg/github/repositories.go

@@ -768,33 +768,32 @@ func GetFileContents(t translations.TranslationHelperFunc) inventory.ServerTool
 						strings.HasSuffix(contentType, "+json") ||
 						strings.HasSuffix(contentType, "+xml")
 
+					// Check if embedded resources should be disabled
+					disableEmbedded := deps.IsFeatureEnabled(ctx, FeatureFlagDisableEmbeddedResources)
+
 					if isTextContent {
 						result := &mcp.ResourceContents{
 							URI:      resourceURI,
 							Text:     string(body),
 							MIMEType: contentType,
 						}
-						// Check if embedded resources should be disabled
-						disableEmbedded := deps.IsFeatureEnabled(ctx, FeatureFlagDisableEmbeddedResources)
 						// Include SHA in the result metadata
 						if fileSHA != "" {
-							return utils.NewToolResultResourceWithFlag(fmt.Sprintf("successfully downloaded text file (SHA: %s)", fileSHA)+successNote, result, disableEmbedded), nil, nil
+							return utils.NewToolResultResource(fmt.Sprintf("successfully downloaded text file (SHA: %s)", fileSHA)+successNote, result, disableEmbedded), nil, nil
 						}
-						return utils.NewToolResultResourceWithFlag("successfully downloaded text file"+successNote, result, disableEmbedded), nil, nil
+						return utils.NewToolResultResource("successfully downloaded text file"+successNote, result, disableEmbedded), nil, nil
 					}
 
 					result := &mcp.ResourceContents{
 						URI:      resourceURI,
 						Blob:     body,
 						MIMEType: contentType,
 					}
-					// Check if embedded resources should be disabled
-					disableEmbedded := deps.IsFeatureEnabled(ctx, FeatureFlagDisableEmbeddedResources)
 					// Include SHA in the result metadata
 					if fileSHA != "" {
-						return utils.NewToolResultResourceWithFlag(fmt.Sprintf("successfully downloaded binary file (SHA: %s)", fileSHA)+successNote, result, disableEmbedded), nil, nil
+						return utils.NewToolResultResource(fmt.Sprintf("successfully downloaded binary file (SHA: %s)", fileSHA)+successNote, result, disableEmbedded), nil, nil
 					}
-					return utils.NewToolResultResourceWithFlag("successfully downloaded binary file"+successNote, result, disableEmbedded), nil, nil
+					return utils.NewToolResultResource("successfully downloaded binary file"+successNote, result, disableEmbedded), nil, nil
 				}
 
 				// Raw API call failed

pkg/utils/result.go

@@ -36,35 +36,31 @@ func NewToolResultErrorFromErr(message string, err error) *mcp.CallToolResult {
 	}
 }
 
-func NewToolResultResource(message string, contents *mcp.ResourceContents) *mcp.CallToolResult {
-	return &mcp.CallToolResult{
-		Content: []mcp.Content{
-			&mcp.TextContent{
-				Text: message,
-			},
-			&mcp.EmbeddedResource{
-				Resource: contents,
-			},
-		},
-		IsError: false,
-	}
-}
-
-// NewToolResultResourceWithFlag returns a CallToolResult with either an embedded resource
+// NewToolResultResource returns a CallToolResult with either an embedded resource
 // or regular content based on the disableEmbeddedResources flag.
 // When disableEmbeddedResources is true, text content is returned as TextContent and
 // binary content is returned as ImageContent, providing better client compatibility.
-func NewToolResultResourceWithFlag(message string, contents *mcp.ResourceContents, disableEmbeddedResources bool) *mcp.CallToolResult {
+func NewToolResultResource(message string, contents *mcp.ResourceContents, disableEmbeddedResources bool) *mcp.CallToolResult {
 	if !disableEmbeddedResources {
 		// Default behavior - return as embedded resource
-		return NewToolResultResource(message, contents)
+		return &mcp.CallToolResult{
+			Content: []mcp.Content{
+				&mcp.TextContent{
+					Text: message,
+				},
+				&mcp.EmbeddedResource{
+					Resource: contents,
+				},
+			},
+			IsError: false,
+		}
 	}
 
 	// When flag is enabled, return as regular content
 	var content mcp.Content
 	switch {
 	case contents.Text != "":
-		// Text content - use TextContent with mime type in annotations
+		// Text content - use TextContent with MIME type in metadata
 		content = &mcp.TextContent{
 			Text: contents.Text,
 			Annotations: &mcp.Annotations{
@@ -76,7 +72,7 @@ func NewToolResultResourceWithFlag(message string, contents *mcp.ResourceContent
 			},
 		}
 	case len(contents.Blob) > 0:
-		// Binary content - use ImageContent with base64 data
+		// Binary content - use ImageContent with raw binary data
 		// Note: MCP SDK will handle base64 encoding during JSON marshaling
 		content = &mcp.ImageContent{
 			Data:     contents.Blob,
@@ -90,7 +86,17 @@ func NewToolResultResourceWithFlag(message string, contents *mcp.ResourceContent
 		}
 	default:
 		// Fallback to embedded resource if neither text nor blob
-		return NewToolResultResource(message, contents)
+		return &mcp.CallToolResult{
+			Content: []mcp.Content{
+				&mcp.TextContent{
+					Text: message,
+				},
+				&mcp.EmbeddedResource{
+					Resource: contents,
+				},
+			},
+			IsError: false,
+		}
 	}
 
 	return &mcp.CallToolResult{

pkg/utils/result_test.go

@@ -8,15 +8,15 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-func TestNewToolResultResourceWithFlag_DisabledFlag(t *testing.T) {
+func TestNewToolResultResource_DisabledFlag(t *testing.T) {
 	// When flag is disabled, should return embedded resource (default behavior)
 	contents := &mcp.ResourceContents{
 		URI:      "test://file.txt",
 		Text:     "Hello, World!",
 		MIMEType: "text/plain",
 	}
 
-	result := NewToolResultResourceWithFlag("Test message", contents, false)
+	result := NewToolResultResource("Test message", contents, false)
 
 	require.NotNil(t, result)
 	require.Len(t, result.Content, 2)
@@ -35,15 +35,15 @@ func TestNewToolResultResourceWithFlag_DisabledFlag(t *testing.T) {
 	assert.Equal(t, contents.MIMEType, embeddedResource.Resource.MIMEType)
 }
 
-func TestNewToolResultResourceWithFlag_EnabledFlag_TextContent(t *testing.T) {
+func TestNewToolResultResource_EnabledFlag_TextContent(t *testing.T) {
 	// When flag is enabled with text content, should return TextContent
 	contents := &mcp.ResourceContents{
 		URI:      "test://file.txt",
 		Text:     "Hello, World!",
 		MIMEType: "text/plain",
 	}
 
-	result := NewToolResultResourceWithFlag("Test message", contents, true)
+	result := NewToolResultResource("Test message", contents, true)
 
 	require.NotNil(t, result)
 	require.Len(t, result.Content, 2)
@@ -65,7 +65,7 @@ func TestNewToolResultResourceWithFlag_EnabledFlag_TextContent(t *testing.T) {
 	assert.Contains(t, textContent.Annotations.Audience, mcp.Role("user"))
 }
 
-func TestNewToolResultResourceWithFlag_EnabledFlag_BinaryContent(t *testing.T) {
+func TestNewToolResultResource_EnabledFlag_BinaryContent(t *testing.T) {
 	// When flag is enabled with binary content, should return ImageContent
 	binaryData := []byte{0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A} // PNG header
 	contents := &mcp.ResourceContents{
@@ -74,7 +74,7 @@ func TestNewToolResultResourceWithFlag_EnabledFlag_BinaryContent(t *testing.T) {
 		MIMEType: "image/png",
 	}
 
-	result := NewToolResultResourceWithFlag("Binary message", contents, true)
+	result := NewToolResultResource("Binary message", contents, true)
 
 	require.NotNil(t, result)
 	require.Len(t, result.Content, 2)
@@ -98,14 +98,14 @@ func TestNewToolResultResourceWithFlag_EnabledFlag_BinaryContent(t *testing.T) {
 	assert.Contains(t, imageContent.Annotations.Audience, mcp.Role("user"))
 }
 
-func TestNewToolResultResourceWithFlag_EnabledFlag_EmptyContent(t *testing.T) {
+func TestNewToolResultResource_EnabledFlag_EmptyContent(t *testing.T) {
 	// When flag is enabled but neither text nor blob exists, should fallback to embedded resource
 	contents := &mcp.ResourceContents{
 		URI:      "test://empty",
 		MIMEType: "application/octet-stream",
 	}
 
-	result := NewToolResultResourceWithFlag("Empty message", contents, true)
+	result := NewToolResultResource("Empty message", contents, true)
 
 	require.NotNil(t, result)
 	require.Len(t, result.Content, 2)