Skip to content

Commit bd075b0

Browse files
authored
Deduplicate raw MCP text envelope construction with shared helper (#4352)
Raw MCP text-content envelopes (`{"content":[{"type":"text","text":"..."}]}`) were being hand-constructed in multiple production paths, creating drift risk if the envelope shape changes. This PR centralizes that shape behind one helper and updates existing call sites to use it. - **Shared MCP response helper** - Added `mcp.BuildMCPTextResponse(text string) map[string]interface{}` in `internal/mcp/tool_result.go`. - Establishes a single source of truth for raw text envelope shape used by guard-facing/raw-map flows. - **Call site consolidation** - Replaced inline envelope maps in: - `internal/server/unified.go` (`callCollaboratorPermission`) - `internal/proxy/proxy.go` (`restBackendCaller`) - `internal/server/system_tools.go` (`sysInit`) - `internal/server/system_tools.go` (`listServers`) - **Targeted unit coverage** - Added `TestBuildMCPTextResponse` in `internal/mcp/tool_result_test.go` to validate envelope structure and text passthrough. ```go func BuildMCPTextResponse(text string) map[string]interface{} { return map[string]interface{}{ "content": []map[string]interface{}{ {"type": "text", "text": text}, }, } } ``` > [!WARNING] > > <details> > <summary>Firewall rules blocked me from connecting to one or more addresses (expand for details)</summary> > > #### I tried to connect to the following addresses, but was blocked by firewall rules: > > - `example.com` > - Triggering command: `/tmp/go-build3674478355/b509/launcher.test /tmp/go-build3674478355/b509/launcher.test -test.testlogfile=/tmp/go-build3674478355/b509/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true rotocol/go-sdk@v1.5.0/internal/json/json.go -I x_amd64/vet --gdwarf-5 --64 ut-4038412272.c x_amd64/vet -I g_.a -fPIC x_amd64/vet -pthread go-sdk/oauthex -fmessage-length-bool x_amd64/vet` (dns block) > - Triggering command: `/tmp/go-build2111533505/b513/launcher.test /tmp/go-build2111533505/b513/launcher.test -test.testlogfile=/tmp/go-build2111533505/b513/testlog.txt -test.paniconexit0 -test.timeout=10m0s` (dns block) > - `invalid-host-that-does-not-exist-12345.com` > - Triggering command: `/tmp/go-build3674478355/b491/config.test /tmp/go-build3674478355/b491/config.test -test.testlogfile=/tmp/go-build3674478355/b491/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true /internal/httpcommon/ascii.go /internal/httpcommon/headermap.go x_amd64/vet --gdwarf-5 ternal/engine/in-atomic -o x_amd64/vet 2897�� g_.a -I x_amd64/vet --gdwarf-5` (dns block) > - Triggering command: `/tmp/go-build2111533505/b495/config.test /tmp/go-build2111533505/b495/config.test -test.testlogfile=/tmp/go-build2111533505/b495/testlog.txt -test.paniconexit0 -test.timeout=10m0s 2897�� g_.a -goversion 64/pkg/tool/linux_amd64/vet -c=4 -nolocalimports -importcfg 64/pkg/tool/linux_amd64/vet -uns�� q62FCnbEi 64/pkg/tool/linu-I 64/pkg/tool/linux_amd64/vet g_.a amVETxCjQ ache/go/1.25.9/x--noprofile 64/pkg/tool/linux_amd64/vet` (dns block) > - `nonexistent.local` > - Triggering command: `/tmp/go-build3674478355/b509/launcher.test /tmp/go-build3674478355/b509/launcher.test -test.testlogfile=/tmp/go-build3674478355/b509/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true rotocol/go-sdk@v1.5.0/internal/json/json.go -I x_amd64/vet --gdwarf-5 --64 ut-4038412272.c x_amd64/vet -I g_.a -fPIC x_amd64/vet -pthread go-sdk/oauthex -fmessage-length-bool x_amd64/vet` (dns block) > - Triggering command: `/tmp/go-build2111533505/b513/launcher.test /tmp/go-build2111533505/b513/launcher.test -test.testlogfile=/tmp/go-build2111533505/b513/testlog.txt -test.paniconexit0 -test.timeout=10m0s` (dns block) > - `slow.example.com` > - Triggering command: `/tmp/go-build3674478355/b509/launcher.test /tmp/go-build3674478355/b509/launcher.test -test.testlogfile=/tmp/go-build3674478355/b509/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true rotocol/go-sdk@v1.5.0/internal/json/json.go -I x_amd64/vet --gdwarf-5 --64 ut-4038412272.c x_amd64/vet -I g_.a -fPIC x_amd64/vet -pthread go-sdk/oauthex -fmessage-length-bool x_amd64/vet` (dns block) > - Triggering command: `/tmp/go-build2111533505/b513/launcher.test /tmp/go-build2111533505/b513/launcher.test -test.testlogfile=/tmp/go-build2111533505/b513/testlog.txt -test.paniconexit0 -test.timeout=10m0s` (dns block) > - `this-host-does-not-exist-12345.com` > - Triggering command: `/tmp/go-build3674478355/b518/mcp.test /tmp/go-build3674478355/b518/mcp.test -test.testlogfile=/tmp/go-build3674478355/b518/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true olang.org/grpc@v1.80.0/internal/transport/bdp_estimator.go olang.org/grpc@v1.80.0/internal/transport/client_stream.go x_amd64/vet --gdwarf-5 g/grpc/metadata -o x_amd64/vet .cfg�� 2897549/b447/_pkg_.a -trimpath x_amd64/vet -p contextprotocol//usr/bin/runc -lang=go1.25 x_amd64/vet` (dns block) > - Triggering command: `/tmp/go-build3099190320/b253/mcp.test /tmp/go-build3099190320/b253/mcp.test -test.testlogfile=/tmp/go-build3099190320/b253/testlog.txt -test.paniconexit0 -test.timeout=10m0s` (dns block) > > If you need me to access, download, or install something from one of these locations, you can either: > > - Configure [Actions setup steps](https://gh.io/copilot/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](https://github.com/github/gh-aw-mcpg/settings/copilot/coding_agent) (admins only) > > </details>
2 parents 80ecf22 + da2c5b7 commit bd075b0

5 files changed

Lines changed: 30 additions & 28 deletions

File tree

internal/mcp/tool_result.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,3 +156,12 @@ func NewErrorCallToolResult(err error) (*sdk.CallToolResult, interface{}, error)
156156
},
157157
}, nil, err
158158
}
159+
160+
// BuildMCPTextResponse returns a raw MCP response map with a single text content item.
161+
func BuildMCPTextResponse(text string) map[string]interface{} {
162+
return map[string]interface{}{
163+
"content": []map[string]interface{}{
164+
{"type": "text", "text": text},
165+
},
166+
}
167+
}

internal/mcp/tool_result_test.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -415,3 +415,18 @@ func TestConvertToCallToolResult_MarshalError(t *testing.T) {
415415
assert.Nil(t, result)
416416
assert.Contains(t, err.Error(), "failed to marshal backend result")
417417
}
418+
419+
func TestBuildMCPTextResponse(t *testing.T) {
420+
text := `{"permission":"write"}`
421+
422+
result := BuildMCPTextResponse(text)
423+
424+
require := require.New(t)
425+
assert := assert.New(t)
426+
427+
content, ok := result["content"].([]map[string]interface{})
428+
require.True(ok)
429+
require.Len(content, 1)
430+
assert.Equal("text", content[0]["type"])
431+
assert.Equal(text, content[0]["text"])
432+
}

internal/proxy/proxy.go

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"github.com/github/gh-aw-mcpg/internal/difc"
2121
"github.com/github/gh-aw-mcpg/internal/guard"
2222
"github.com/github/gh-aw-mcpg/internal/logger"
23+
"github.com/github/gh-aw-mcpg/internal/mcp"
2324
"github.com/github/gh-aw-mcpg/internal/tracing"
2425
)
2526

@@ -331,12 +332,7 @@ func (r *restBackendCaller) CallTool(ctx context.Context, toolName string, args
331332
}
332333

333334
// Wrap in MCP response format: {content: [{type: "text", text: "..."}]}
334-
mcpResp := map[string]interface{}{
335-
"content": []map[string]interface{}{
336-
{"type": "text", "text": string(body)},
337-
},
338-
}
339-
return mcpResp, nil
335+
return mcp.BuildMCPTextResponse(string(body)), nil
340336
}
341337

342338
// forwardToGitHub sends a request to the upstream GitHub API.

internal/server/system_tools.go

Lines changed: 3 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"fmt"
66

77
"github.com/github/gh-aw-mcpg/internal/logger"
8+
"github.com/github/gh-aw-mcpg/internal/mcp"
89
)
910

1011
var logSys = logger.New("server:system_tools")
@@ -89,14 +90,7 @@ func (s *SysServer) callTool(name string, args map[string]interface{}) (interfac
8990

9091
func (s *SysServer) sysInit() (interface{}, error) {
9192
logSys.Printf("Initializing MCPG system with %d servers", len(s.serverIDs))
92-
return map[string]interface{}{
93-
"content": []map[string]interface{}{
94-
{
95-
"type": "text",
96-
"text": fmt.Sprintf("MCPG initialized. Available servers: %v", s.serverIDs),
97-
},
98-
},
99-
}, nil
93+
return mcp.BuildMCPTextResponse(fmt.Sprintf("MCPG initialized. Available servers: %v", s.serverIDs)), nil
10094
}
10195

10296
func (s *SysServer) listServers() (interface{}, error) {
@@ -105,12 +99,5 @@ func (s *SysServer) listServers() (interface{}, error) {
10599
serverList += fmt.Sprintf("%d. %s\n", i+1, id)
106100
}
107101

108-
return map[string]interface{}{
109-
"content": []map[string]interface{}{
110-
{
111-
"type": "text",
112-
"text": fmt.Sprintf("Configured MCP Servers:\n%s", serverList),
113-
},
114-
},
115-
}, nil
102+
return mcp.BuildMCPTextResponse(fmt.Sprintf("Configured MCP Servers:\n%s", serverList)), nil
116103
}

internal/server/unified.go

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -346,12 +346,7 @@ func (g *guardBackendCaller) callCollaboratorPermission(ctx context.Context, arg
346346
}
347347

348348
// Wrap in MCP response format so the WASM guard can parse it consistently
349-
mcpResp := map[string]interface{}{
350-
"content": []map[string]interface{}{
351-
{"type": "text", "text": string(body)},
352-
},
353-
}
354-
return mcpResp, nil
349+
return mcp.BuildMCPTextResponse(string(body)), nil
355350
}
356351

357352
// buildCircuitBreakers creates per-backend circuit breakers from the configuration.

0 commit comments

Comments
 (0)