Refactor: Extract marshalToResponse helper to eliminate duplicate JSON marshal pattern

[Claude]

The internal/mcp/connection.go file contained 6 identical instances of JSON marshalling with error handling across all MCP SDK method wrappers (listTools, callTool, listResources, readResource, listPrompts, getPrompt).

Changes

• Added marshalToResponse helper function that encapsulates the JSON marshal + error check + Response construction pattern
• Updated 6 MCP methods to use the helper, reducing each from ~13 lines to ~3-5 lines
• Added comprehensive test coverage with 8 test cases in marshal_helper_test.go

Impact

• Reduced connection.go by 39 lines (3.9% reduction)
• Single point of maintenance for response marshalling logic
• Consistent error messages with wrapped errors (fmt.Errorf)
• No breaking changes - all existing tests pass

Example

Before:

func (c *Connection) listTools() (*Response, error) {
    result, err := c.session.ListTools(c.ctx, &sdk.ListToolsParams{})
    if err != nil {
        return nil, err
    }
    
    resultJSON, err := json.Marshal(result)
    if err != nil {
        return nil, err
    }
    
    return &Response{
        JSONRPC: "2.0",
        ID:      1,
        Result:  resultJSON,
    }, nil
}

After:

func (c *Connection) listTools() (*Response, error) {
    result, err := c.session.ListTools(c.ctx, &sdk.ListToolsParams{})
    if err != nil {
        return nil, err
    }
    
    return marshalToResponse(result)
}

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-build1883682307/b279/launcher.test /tmp/go-build1883682307/b279/launcher.test -test.testlogfile=/tmp/go-build1883682307/b279/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true /mcp/connection.-p 64/pkg/tool/linutext/template cal/bin/git go !.git /opt/hostedtoolc/tmp/go-build2623697467/b138/vet.cfg git rev-�� --abbrev-ref HEAD rgo/bin/git 64/src/runtime/c/opt/hostedtoolcache/go/1.25.7/x64/pkg/tool/linux_amd64/vet irements.go ache/go/1.25.7/x-pack base64 (dns block)
  • Triggering command: /tmp/go-build4228194561/b279/launcher.test /tmp/go-build4228194561/b279/launcher.test -test.testlogfile=/tmp/go-build4228194561/b279/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true --abbrev-ref HEAD 64/pkg/tool/linu--bundle 6407cb75efd18d88/home/REDACTED/work/gh-aw-mcpg/gh-aw-mcpg/awmg -trimpath ache/go/1.25.7/x--listen 64/pkg/tool/linu127.0.0.1:13099 (dns block)
  • Triggering command: /tmp/go-build1470814238/b279/launcher.test /tmp/go-build1470814238/b279/launcher.test -test.testlogfile=/tmp/go-build1470814238/b279/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true aw-mcpg/internal/config/config_core.go aw-mcpg/internal/config/config_feature.go -data-downloader /opt/hostedtoolcsleep 3764740-215ubf.s5 64/pkg/tool/linux_amd64/link -data-downloader e=/t�� t0 m0s by/a24f9c864e9356407cb75efd18d882b34a19b7f9c9a1afcc13d2869ff696c0d6/log.json /opt/hostedtoolcbase64 -I /opt/hostedtoolcache/go/1.25.7/x--abbrev-ref docker-buildx (dns block)
• invalid-host-that-does-not-exist-12345.com
  • Triggering command: /tmp/go-build1883682307/b264/config.test /tmp/go-build1883682307/b264/config.test -test.testlogfile=/tmp/go-build1883682307/b264/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true (dns block)
  • Triggering command: /tmp/go-build2052208211/b264/config.test /tmp/go-build2052208211/b264/config.test -test.testlogfile=/tmp/go-build2052208211/b264/testlog.txt -test.paniconexit0 -test.timeout=10m0s -uns�� /tmp/go-build2623697467/b113/vet.cfg -goversion /opt/hostedtoolcache/go/1.25.7/x64/pkg/tool/linux_amd64/vet by/22165225ae4b4sleep -nolocalimports -importcfg /opt/hostedtoolcache/go/1.25.7/x64/pkg/tool/linu--bundle /tmp�� 99a743835c3d5b2b git x_amd64/compile 0, length(NVM_DIbase64 HEAD t x_amd64/compile (dns block)
• nonexistent.local
  • Triggering command: /tmp/go-build1883682307/b279/launcher.test /tmp/go-build1883682307/b279/launcher.test -test.testlogfile=/tmp/go-build1883682307/b279/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true /mcp/connection.-p 64/pkg/tool/linutext/template cal/bin/git go !.git /opt/hostedtoolc/tmp/go-build2623697467/b138/vet.cfg git rev-�� --abbrev-ref HEAD rgo/bin/git 64/src/runtime/c/opt/hostedtoolcache/go/1.25.7/x64/pkg/tool/linux_amd64/vet irements.go ache/go/1.25.7/x-pack base64 (dns block)
  • Triggering command: /tmp/go-build4228194561/b279/launcher.test /tmp/go-build4228194561/b279/launcher.test -test.testlogfile=/tmp/go-build4228194561/b279/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true --abbrev-ref HEAD 64/pkg/tool/linu--bundle 6407cb75efd18d88/home/REDACTED/work/gh-aw-mcpg/gh-aw-mcpg/awmg -trimpath ache/go/1.25.7/x--listen 64/pkg/tool/linu127.0.0.1:13099 (dns block)
  • Triggering command: /tmp/go-build1470814238/b279/launcher.test /tmp/go-build1470814238/b279/launcher.test -test.testlogfile=/tmp/go-build1470814238/b279/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true aw-mcpg/internal/config/config_core.go aw-mcpg/internal/config/config_feature.go -data-downloader /opt/hostedtoolcsleep 3764740-215ubf.s5 64/pkg/tool/linux_amd64/link -data-downloader e=/t�� t0 m0s by/a24f9c864e9356407cb75efd18d882b34a19b7f9c9a1afcc13d2869ff696c0d6/log.json /opt/hostedtoolcbase64 -I /opt/hostedtoolcache/go/1.25.7/x--abbrev-ref docker-buildx (dns block)
• slow.example.com
  • Triggering command: /tmp/go-build1883682307/b279/launcher.test /tmp/go-build1883682307/b279/launcher.test -test.testlogfile=/tmp/go-build1883682307/b279/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true /mcp/connection.-p 64/pkg/tool/linutext/template cal/bin/git go !.git /opt/hostedtoolc/tmp/go-build2623697467/b138/vet.cfg git rev-�� --abbrev-ref HEAD rgo/bin/git 64/src/runtime/c/opt/hostedtoolcache/go/1.25.7/x64/pkg/tool/linux_amd64/vet irements.go ache/go/1.25.7/x-pack base64 (dns block)
  • Triggering command: /tmp/go-build4228194561/b279/launcher.test /tmp/go-build4228194561/b279/launcher.test -test.testlogfile=/tmp/go-build4228194561/b279/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true --abbrev-ref HEAD 64/pkg/tool/linu--bundle 6407cb75efd18d88/home/REDACTED/work/gh-aw-mcpg/gh-aw-mcpg/awmg -trimpath ache/go/1.25.7/x--listen 64/pkg/tool/linu127.0.0.1:13099 (dns block)
  • Triggering command: /tmp/go-build1470814238/b279/launcher.test /tmp/go-build1470814238/b279/launcher.test -test.testlogfile=/tmp/go-build1470814238/b279/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true aw-mcpg/internal/config/config_core.go aw-mcpg/internal/config/config_feature.go -data-downloader /opt/hostedtoolcsleep 3764740-215ubf.s5 64/pkg/tool/linux_amd64/link -data-downloader e=/t�� t0 m0s by/a24f9c864e9356407cb75efd18d882b34a19b7f9c9a1afcc13d2869ff696c0d6/log.json /opt/hostedtoolcbase64 -I /opt/hostedtoolcache/go/1.25.7/x--abbrev-ref docker-buildx (dns block)
• this-host-does-not-exist-12345.com
  • Triggering command: /tmp/go-build2086996096/b001/mcp.test /tmp/go-build2086996096/b001/mcp.test -test.testlogfile=/tmp/go-build2086996096/b001/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true p=/opt/hostedtoolcache/go/1.25.7/x64=/_/GOROOT .cfg de/node/bin/git --gdwarf-5 --64 -o /usr/libexec/gccHEAD 7223�� ache/go/1.25.7/x64/src/net /opt/hostedtoolcache/go/1.25.7/x64/src/runtime/cgo ache/go/1.25.7/x64/pkg/tool/linux_amd64/vet /tmp/go-build343base64 -imultiarch x86_64-linux-gnu ache/go/1.25.7/x64/pkg/tool/linuHEAD (dns block)
  • Triggering command: /tmp/go-build2052208211/b288/mcp.test /tmp/go-build2052208211/b288/mcp.test -test.testlogfile=/tmp/go-build2052208211/b288/testlog.txt -test.paniconexit0 -test.timeout=10m0s -tes�� -test.paniconexit0 -test.timeout=10m0s k/gh-aw-mcpg/gh-aw-mcpg/awmg 86_64/git 64/pkg/tool/linu-V=full git 0d6/log.json /usr�� y shot-bash-1771073764740-215ubf.sh && { shopt -u extglob || setopt NO_EXTENDED_GLOB; } 2>/dev/nuldocker-cli-plugin-metadata ntime.v2.task/moby/22165225ae4b4e4709b6ad656878452079d16798774077c299a743835c3d5b2b/log.json --abbrev-ref 0d6 /usr/bin/base64 0d6/init.pid (dns block)

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

• Configure 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 (admins only)

Original prompt

---

This section details on the original issue you should resolve

<issue_title>[duplicate-code] Duplicate Code Pattern: JSON Marshal + Error Check in MCP Connection</issue_title>
<issue_description># 🔍 Duplicate Code Pattern: JSON Marshal + Error Check

Part of duplicate code analysis: #892

Summary

The internal/mcp/connection.go file contains 6 identical instances of a JSON marshalling pattern with error handling. This pattern appears in all MCP SDK method wrappers (listTools, callTool, listResources, readResource, listPrompts, getPrompt).

Duplication Details

Pattern: JSON Marshal Result + Error Check

• Severity: Medium

• Occurrences: 6 instances in connection.go

• Locations:

  • internal/mcp/connection.go (lines 808-811) - listTools method
  • internal/mcp/connection.go (lines 851-854) - callTool method
  • internal/mcp/connection.go (lines 872-875) - listResources method
  • internal/mcp/connection.go (lines 903-906) - readResource method
  • internal/mcp/connection.go (lines 924-927) - listPrompts method
  • internal/mcp/connection.go (lines 957-960) - getPrompt method

• Code Sample:

// Duplicated pattern (appears 6 times):
resultJSON, err := json.Marshal(result)
if err != nil {
    return nil, err
}

return &Response{
    Result: (*json.RawMessage)(&resultJSON),
}

Impact Analysis

• Maintainability: Any change to error handling or response construction requires updating 6 locations
• Bug Risk: High potential for inconsistent error messages or logging if updates are applied unevenly
• Code Bloat: ~18 lines of duplicated code (3 lines × 6 occurrences)
• Consistency: All 6 methods currently have identical behavior, suggesting they share the same responsibility

Refactoring Recommendations

1. Extract Helper Function: marshalToResponse

• Create a shared helper function in internal/mcp/connection.go:

// marshalToResponse marshals an SDK result into a Response object
func marshalToResponse(result interface{}) (*Response, error) {
    resultJSON, err := json.Marshal(result)
    if err != nil {
        return nil, fmt.Errorf("failed to marshal result: %w", err)
    }
    
    return &Response{
        Result: (*json.RawMessage)(&resultJSON),
    }, nil
}

• Update all 6 methods to use: return marshalToResponse(result)
• Estimated effort: 1-2 hours
• Benefits:
  • Single point of maintenance for response marshalling
  • Consistent error messages across all methods
  • Easier to add logging or debugging if needed
  • Reduces file from 1,011 lines to ~999 lines

2. Alternative: Generic Response Wrapper

• If future needs require more complex response processing, consider a more flexible wrapper:

func (c *Connection) wrapSDKResponse(method string, sdkCall func() (interface{}, error)) (*Response, error) {
    result, err := sdkCall()
    if err != nil {
        return nil, err
    }
    return marshalToResponse(result)
}

• This would allow adding method-specific logging or instrumentation
• Estimated effort: 2-3 hours
• Benefits: More flexible for future enhancements

Implementation Checklist

• Review duplication findings with team
• Choose refactoring approach (Option 1 recommended for simplicity)
• Create helper function marshalToResponse in connection.go
• Update all 6 MCP method wrappers to use the helper
• Verify no functionality broken (run existing tests)
• Consider adding unit test for the new helper function
• Update any relevant code comments

Parent Issue

See parent analysis report: #892
Related to #892

AI generated by Duplicate Code Detector

• expires on Feb 18, 2026, 10:26 AM UTC

Comments on the Issue (you are @claude[agent] in this section)

• Fixes [duplicate-code] Duplicate Code Pattern: JSON Marshal + Error Check in MCP Connection #894

[Copilot]

Pull request overview

This PR successfully refactors duplicate JSON marshalling logic in the MCP connection layer by extracting a reusable helper function. The change eliminates 6 identical code blocks across all MCP SDK method wrappers, improving maintainability without introducing any breaking changes.

Changes:

• Extracted marshalToResponse helper function that encapsulates JSON marshal + error check + Response construction pattern
• Refactored 6 MCP SDK method wrappers (listTools, callTool, listResources, readResource, listPrompts, getPrompt) to use the new helper
• Added comprehensive test coverage with 8 test cases covering success paths, edge cases, and error scenarios

Reviewed changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated no comments.

File	Description
internal/mcp/connection.go	Added marshalToResponse helper function and refactored 6 MCP method wrappers to use it, reducing duplication by 39 lines
internal/mcp/marshal_helper_test.go	Added comprehensive test suite with 8 test cases covering various scenarios including simple/complex marshalling, nil/empty inputs, and unmarshalable types
main.go	Removed trailing whitespace on line 16

---

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