Skip to content

Deduplicate raw TextContent fallback in tool result conversion#5241

Merged
lpcox merged 2 commits into
mainfrom
copilot/fix-duplicate-code-pattern-again
May 7, 2026
Merged

Deduplicate raw TextContent fallback in tool result conversion#5241
lpcox merged 2 commits into
mainfrom
copilot/fix-duplicate-code-pattern-again

Conversation

Copilot AI commented May 7, 2026

Copy link
Copy Markdown
Contributor

internal/mcp/tool_result.go repeated the same marshal-to-TextContent fallback in four branches of ConvertToCallToolResult / convertMapToCallToolResult. This change centralizes that behavior so fallback handling stays consistent as the conversion logic evolves.

  • Refactor fallback construction

    • Extract marshalValueToTextContentResult to own the shared:
      • json.Marshal(...)
      • sdk.CallToolResult{Content: []sdk.Content{&sdk.TextContent{...}}}
      • error wrapping on marshal failure
    • Replace the four duplicated fallback blocks with calls to the helper
  • Behavior preserved across existing fallback paths

    • direct array input ([]interface{})
    • scalar / non-structured input
    • map input with missing or nil content
    • map input with unrecognized content shape
  • Focused coverage

    • Add a unit test for the content: nil map case to ensure it still falls back to raw text wrapping
func marshalValueToTextContentResult(value interface{}) (*sdk.CallToolResult, error) {
	dataBytes, err := json.Marshal(value)
	if err != nil {
		return nil, fmt.Errorf("failed to marshal backend result: %w", err)
	}
	return &sdk.CallToolResult{
		Content: []sdk.Content{&sdk.TextContent{Text: string(dataBytes)}},
	}, nil
}

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • example.com
    • Triggering command: /tmp/go-build1542213297/b513/launcher.test /tmp/go-build1542213297/b513/launcher.test -test.testlogfile=/tmp/go-build1542213297/b513/testlog.txt -test.paniconexit0 -test.timeout=10m0s /tmp/go-build1542213297/b445/vet.cfg g_.a -I x_amd64/vet --gdwarf-5 ls/insecure -o x_amd64/vet -qui�� .cfg /tmp/go-build418-ifaceassert x_amd64/vet . .io/otel/semconv-atomic x86_64-linux-gnu-bool x_amd64/vet (dns block)
    • Triggering command: /tmp/go-build2637788630/b513/launcher.test /tmp/go-build2637788630/b513/launcher.test -test.testlogfile=/tmp/go-build2637788630/b513/testlog.txt -test.paniconexit0 -test.timeout=10m0s .o .o .o .o .o .o f07e34dca1.build--exclude-standard lib/rustlib/x86_--others lib/�� lib/rustlib/x86_64-REDACTED-linux-gnu/lib/libobject-926daa94a00ee327.rlib lib/rustlib/x86_64-REDACTED-linux-gnu/lib/libmemchr-48d5b0db80402653.rlib lib/rustlib/x86_64-REDACTED-linux-gnu/lib/libaddr2line-3367f26bd486b29d.rlib (compatibility issue with Go 1.25.0). Continuing with other checks..."; \ elif command -v golan lib/rustlib/x86_--norc lib/rustlib/x86_--noprofile 05ed-cgu.00.rcgu.o (dns block)
  • invalid-host-that-does-not-exist-12345.com
    • Triggering command: /tmp/go-build1542213297/b495/config.test /tmp/go-build1542213297/b495/config.test -test.testlogfile=/tmp/go-build1542213297/b495/testlog.txt -test.paniconexit0 -test.timeout=10m0s /tmp/go-build1542213297/b393/vet.cfg uf@v1.36.11/protoadapt/convert.go om/tetratelabs/wazero@v1.11.0/internal/fsapi/polgoogle.golang.org/grpc/balancer/pickfirst/intern-atomic x_amd64/vet 4460836/b150/ --64 (dns block)
  • nonexistent.local
    • Triggering command: /tmp/go-build1542213297/b513/launcher.test /tmp/go-build1542213297/b513/launcher.test -test.testlogfile=/tmp/go-build1542213297/b513/testlog.txt -test.paniconexit0 -test.timeout=10m0s /tmp/go-build1542213297/b445/vet.cfg g_.a -I x_amd64/vet --gdwarf-5 ls/insecure -o x_amd64/vet -qui�� .cfg /tmp/go-build418-ifaceassert x_amd64/vet . .io/otel/semconv-atomic x86_64-linux-gnu-bool x_amd64/vet (dns block)
    • Triggering command: /tmp/go-build2637788630/b513/launcher.test /tmp/go-build2637788630/b513/launcher.test -test.testlogfile=/tmp/go-build2637788630/b513/testlog.txt -test.paniconexit0 -test.timeout=10m0s .o .o .o .o .o .o f07e34dca1.build--exclude-standard lib/rustlib/x86_--others lib/�� lib/rustlib/x86_64-REDACTED-linux-gnu/lib/libobject-926daa94a00ee327.rlib lib/rustlib/x86_64-REDACTED-linux-gnu/lib/libmemchr-48d5b0db80402653.rlib lib/rustlib/x86_64-REDACTED-linux-gnu/lib/libaddr2line-3367f26bd486b29d.rlib (compatibility issue with Go 1.25.0). Continuing with other checks..."; \ elif command -v golan lib/rustlib/x86_--norc lib/rustlib/x86_--noprofile 05ed-cgu.00.rcgu.o (dns block)
  • slow.example.com
    • Triggering command: /tmp/go-build1542213297/b513/launcher.test /tmp/go-build1542213297/b513/launcher.test -test.testlogfile=/tmp/go-build1542213297/b513/testlog.txt -test.paniconexit0 -test.timeout=10m0s /tmp/go-build1542213297/b445/vet.cfg g_.a -I x_amd64/vet --gdwarf-5 ls/insecure -o x_amd64/vet -qui�� .cfg /tmp/go-build418-ifaceassert x_amd64/vet . .io/otel/semconv-atomic x86_64-linux-gnu-bool x_amd64/vet (dns block)
    • Triggering command: /tmp/go-build2637788630/b513/launcher.test /tmp/go-build2637788630/b513/launcher.test -test.testlogfile=/tmp/go-build2637788630/b513/testlog.txt -test.paniconexit0 -test.timeout=10m0s .o .o .o .o .o .o f07e34dca1.build--exclude-standard lib/rustlib/x86_--others lib/�� lib/rustlib/x86_64-REDACTED-linux-gnu/lib/libobject-926daa94a00ee327.rlib lib/rustlib/x86_64-REDACTED-linux-gnu/lib/libmemchr-48d5b0db80402653.rlib lib/rustlib/x86_64-REDACTED-linux-gnu/lib/libaddr2line-3367f26bd486b29d.rlib (compatibility issue with Go 1.25.0). Continuing with other checks..."; \ elif command -v golan lib/rustlib/x86_--norc lib/rustlib/x86_--noprofile 05ed-cgu.00.rcgu.o (dns block)
  • this-host-does-not-exist-12345.com
    • Triggering command: /tmp/go-build1542213297/b522/mcp.test /tmp/go-build1542213297/b522/mcp.test -test.testlogfile=/tmp/go-build1542213297/b522/testlog.txt -test.paniconexit0 -test.timeout=10m0s -o .cfg -trimpath x_amd64/vet -p vendor/golang.or/usr/bin/runc -lang=go1.25 x_amd64/vet .cfg�� 4460836/b392/_pkg_.a -I x_amd64/vet --gdwarf-5 g/protobuf/inter-o -o x_amd64/vet (dns block)
    • Triggering command: /tmp/go-build495322174/b001/mcp.test /tmp/go-build495322174/b001/mcp.test -test.testlogfile=/tmp/go-build495322174/b001/testlog.txt -test.paniconexit0 -test.timeout=10m0s /home/REDACTED/work/gh-aw-mcpg/gh-aw-mcpg/guards/github-guard/rust-guard/target/debug/deps/serde_derive-bdc7cd22a58a5141.serde_derive.12123747d8da05ed-cgu.07.rcgu.o /home/REDACTED/work/gh-aw-mcpg/gh-aw-mcpg/guards/github-guard/rust-guard/target/debug/deps/serde_derive-bdc7cd22a58a5141.serde_derive.12123747d8da05ed-cgu.08.rcgu.o /home/REDACTED/work/gh-aw-mcpg/gh-aw-mcpg/guards/github-guard/rust-guard/target/debug/deps/serde_derive-bdc7cd22a58a5141.serde_derive.12123747d8da05ed-cgu.09.rcgu.o /home/REDACTED/work/gh-aw-mcpg/gh-aw-mcpg/guards/github-guard/rust-guard/target/debug/deps/serde_derive-bdc7cd22a58a5141.serde_derive.12123747d8da05ed-cgu.10.rcgu.o /home/REDACTED/work/gh-aw-mcpg/gh-aw-mcpg/guards/github-guard/rust-guard/target/debug/deps/serde_derive-bdc7cd22a58a5141.serde_derive.12123747d8da05ed-cgu.11.rcgu.o /home/REDACTED/work/gh-aw-mcpg/gh-aw-mcpg/guards/github-guard/rust-guard/target/debug/�� /home/REDACTED/work/gh-aw-mcpg/gh-aw-mcpg/guards/github-guard/rust-guard/target/debug/deps/serde_derive-bdc7cd22a58a5141.serde_derive.12123747d8da05ed-cgu.13.rcgu.o /home/REDACTED/work/gh-aw-mcpg/gh-aw-mcpg/guards/github-guard/rust-guard/target/debug/deps/serde_derive-bdc7cd22a58a5141.serde_derive.12123747d8da05ed-cgu.14.rcgu.o 64-u�� 64-REDACTED-linux-gnu/lib/libminiz_oxide-2b6a8d2f6e1dc71b.rlib 64-REDACTED-linux-gnu/lib/libadler2-39ffdbc27c978ccc.rlib /usr/libexec/doc-Wl,-Bstatic by/405f41b9e5ffacc 42ce6344f8114eba-Wl,--version-script=/home/REDACTED/work/gh-aw-mcpg/gh-aw-mcpg/guards/github-guard/rust-guard/target/debug/deps/rustcsdpj1U/list -importcfg 0cd/log.json (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Copilot AI changed the title [WIP] Fix duplicate code pattern in marshal-to-TextContent fallback Deduplicate raw TextContent fallback in tool result conversion May 7, 2026
Copilot AI requested a review from lpcox May 7, 2026 15:44
Copilot finished work on behalf of lpcox May 7, 2026 15:44
@lpcox lpcox marked this pull request as ready for review May 7, 2026 15:48
Copilot AI review requested due to automatic review settings May 7, 2026 15:48

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Refactors internal/mcp/tool_result.go to centralize the “marshal backend value and wrap as a single TextContent” fallback used by ConvertToCallToolResult and convertMapToCallToolResult, keeping fallback behavior consistent while reducing duplication.

Changes:

  • Extracts repeated marshal-and-wrap fallback into marshalValueToTextContentResult.
  • Replaces four duplicated fallback blocks with the helper.
  • Adds a unit test covering the map[string]interface{}{ "content": nil, ... } fallback case.
Show a summary per file
File Description
internal/mcp/tool_result.go Introduces marshalValueToTextContentResult and routes existing fallback branches through it.
internal/mcp/tool_result_test.go Adds coverage for the content: nil map fallback behavior.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 2/2 changed files
  • Comments generated: 0

@lpcox lpcox merged commit c2bda91 into main May 7, 2026
31 of 32 checks passed
@lpcox lpcox deleted the copilot/fix-duplicate-code-pattern-again branch May 7, 2026 15:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[duplicate-code] Duplicate Code Pattern: Marshal-to-TextContent Fallback in mcp/tool_result.go

3 participants