Skip to content

refactor: extract duplicate code patterns into helpers#1855

Merged
lpcox merged 2 commits intomainfrom
claude/fix-duplicate-code-patterns
Mar 13, 2026
Merged

refactor: extract duplicate code patterns into helpers#1855
lpcox merged 2 commits intomainfrom
claude/fix-duplicate-code-patterns

Conversation

@Claude
Copy link
Contributor

@Claude Claude AI commented Mar 13, 2026

Addresses duplicate code analysis identifying ~50 lines of structural duplication across two patterns in internal/server/unified.go.

Changes

Pattern 1: Backend Tool Call Execution (~40 lines eliminated)

Extracted executeBackendToolCall helper consolidating:

  • Connection management via launcher.GetOrLaunchForSession
  • tools/call request dispatch
  • Backend error checking
  • Result unmarshaling

Before:

// guardBackendCaller.CallTool and callBackendTool Phase 3 each had:
conn, err := launcher.GetOrLaunchForSession(us.launcher, serverID, sessionID)
if err != nil { ... }
response, err := conn.SendRequestWithServerID(ctx, "tools/call", ...)
if err != nil { ... }
if response.Error != nil { ... }
var result interface{}
json.Unmarshal(response.Result, &result)

After:

result, err := executeBackendToolCall(ctx, us.launcher, serverID, sessionID, toolName, args)

Pattern 2: Sys Tool Delegation (~10 lines eliminated)

Extracted callSysServer helper consolidating:

  • Tool name marshaling
  • sysServer.HandleRequest delegation

Before:

// sysInitHandler and sysListHandler each had:
params, _ := json.Marshal(map[string]interface{}{
    "name": toolName,
    "arguments": map[string]interface{}{},
})
result, err := us.sysServer.HandleRequest("tools/call", json.RawMessage(params))

After:

result, err := us.callSysServer("sys_init")

Impact

  • Eliminates copy-paste error risk when modifying backend call or sys server logic
  • Follows existing helper patterns (newErrorCallToolResult, parseToolArguments)
  • No functional changes—pure refactoring

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-build4242760615/b336/launcher.test /tmp/go-build4242760615/b336/launcher.test -test.testlogfile=/tmp/go-build4242760615/b336/testlog.txt -test.paniconexit0 -test.timeout=10m0s -uns�� ache/go/1.25.7/x64/src/net /tmp/go-build1249286257/b024/vet.cfg ache/go/1.25.7/x64/pkg/tool/linux_amd64/vet unset position.go .12/x64/bin/git ache/go/1.25.7/x64/pkg/tool/linuHEAD -W -I /tmp/go-build1249286257/b165/ ache/go/1.25.7/x64/pkg/tool/linux_amd64/compile . --gdwarf2 --64 ache/go/1.25.7/x64/pkg/tool/linux_amd64/compile (dns block)
    • Triggering command: /tmp/go-build2320097585/b332/launcher.test /tmp/go-build2320097585/b332/launcher.test -test.testlogfile=/tmp/go-build2320097585/b332/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true 64/bin/git ortcfg /usr/sbin/bash submodules | heanode 9286257/b132/vet-v ndor/bin/git bash /usr�� runtime-runc/moby ortcfg csi/net-interface-handler aw-mcpg/internal/opt/hostedtoolcache/go/1.25.7/x64/pkg/tool/linux_amd64/asm aw-mcpg/internal-V=full (dns block)
  • invalid-host-that-does-not-exist-12345.com
    • Triggering command: /tmp/go-build4242760615/b318/config.test /tmp/go-build4242760615/b318/config.test -test.testlogfile=/tmp/go-build4242760615/b318/testlog.txt -test.paniconexit0 -test.timeout=10m0s --de�� c.test --debug-prefix-map 64/bin/git -I /opt/hostedtoolc-unsafeptr=false ut-1969789729.c VF78F_aidvBPXKXaKR/S4HB-3OSyH3DKHEAD (dns block)
    • Triggering command: /tmp/go-build2320097585/b314/config.test /tmp/go-build2320097585/b314/config.test -test.testlogfile=/tmp/go-build2320097585/b314/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true 17fb9960acf3cdef -importcfg ine by/b2276789a3c14/usr/bin/chronyc -w -buildmode=exe ine --ve�� 85e4fa5c5748c5dd -extld=gcc /opt/hostedtoolcache/go/1.25.7/xjson aw-mcpg/internalbase64 9286257/b151/vet-d ns /opt/hostedtoolcache/go/1.25.7/x64/pkg/tool/linu--listen (dns block)
  • nonexistent.local
    • Triggering command: /tmp/go-build4242760615/b336/launcher.test /tmp/go-build4242760615/b336/launcher.test -test.testlogfile=/tmp/go-build4242760615/b336/testlog.txt -test.paniconexit0 -test.timeout=10m0s -uns�� ache/go/1.25.7/x64/src/net /tmp/go-build1249286257/b024/vet.cfg ache/go/1.25.7/x64/pkg/tool/linux_amd64/vet unset position.go .12/x64/bin/git ache/go/1.25.7/x64/pkg/tool/linuHEAD -W -I /tmp/go-build1249286257/b165/ ache/go/1.25.7/x64/pkg/tool/linux_amd64/compile . --gdwarf2 --64 ache/go/1.25.7/x64/pkg/tool/linux_amd64/compile (dns block)
    • Triggering command: /tmp/go-build2320097585/b332/launcher.test /tmp/go-build2320097585/b332/launcher.test -test.testlogfile=/tmp/go-build2320097585/b332/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true 64/bin/git ortcfg /usr/sbin/bash submodules | heanode 9286257/b132/vet-v ndor/bin/git bash /usr�� runtime-runc/moby ortcfg csi/net-interface-handler aw-mcpg/internal/opt/hostedtoolcache/go/1.25.7/x64/pkg/tool/linux_amd64/asm aw-mcpg/internal-V=full (dns block)
  • slow.example.com
    • Triggering command: /tmp/go-build4242760615/b336/launcher.test /tmp/go-build4242760615/b336/launcher.test -test.testlogfile=/tmp/go-build4242760615/b336/testlog.txt -test.paniconexit0 -test.timeout=10m0s -uns�� ache/go/1.25.7/x64/src/net /tmp/go-build1249286257/b024/vet.cfg ache/go/1.25.7/x64/pkg/tool/linux_amd64/vet unset position.go .12/x64/bin/git ache/go/1.25.7/x64/pkg/tool/linuHEAD -W -I /tmp/go-build1249286257/b165/ ache/go/1.25.7/x64/pkg/tool/linux_amd64/compile . --gdwarf2 --64 ache/go/1.25.7/x64/pkg/tool/linux_amd64/compile (dns block)
    • Triggering command: /tmp/go-build2320097585/b332/launcher.test /tmp/go-build2320097585/b332/launcher.test -test.testlogfile=/tmp/go-build2320097585/b332/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true 64/bin/git ortcfg /usr/sbin/bash submodules | heanode 9286257/b132/vet-v ndor/bin/git bash /usr�� runtime-runc/moby ortcfg csi/net-interface-handler aw-mcpg/internal/opt/hostedtoolcache/go/1.25.7/x64/pkg/tool/linux_amd64/asm aw-mcpg/internal-V=full (dns block)
  • this-host-does-not-exist-12345.com
    • Triggering command: /tmp/go-build4242760615/b345/mcp.test /tmp/go-build4242760615/b345/mcp.test -test.testlogfile=/tmp/go-build4242760615/b345/testlog.txt -test.paniconexit0 -test.timeout=10m0s -I ache/go/1.25.7/x64/src/net -I if (a[i] && a[i] !~ /^[0-9]+$/) exit(2); if (a[i] < b[i]) exit(3); else if (a[i] > b[i]) exit(0) --gdwarf-5 --64 -o ache/go/1.25.7/x64/pkg/tool/linuHEAD -o /tmp/go-build1249286257/b210/_pkg_.a -trimpath ache/go/1.25.7/x64/pkg/tool/linux_amd64/vet -p github.com/githu/tmp/go-build3632611954/b228/vet.cfg -lang=go1.25 ache/go/1.25.7/x64/pkg/tool/linux_amd64/vet (dns block)
    • Triggering command: /tmp/go-build2320097585/b341/mcp.test /tmp/go-build2320097585/b341/mcp.test -test.testlogfile=/tmp/go-build2320097585/b341/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true /home/REDACTED/work/gh-aw-mcpg/gh-aw-mcpg/internal/strutil/truncat6662819a7d6931901c1593745131476bbash y CURL_CA_BUNDLE=/etc/ssl/certs/ca-certificates.crt", "REQUESTS_CA_BUNDLE=/etc/ssl/c--version by/bd0209c110101/opt/hostedtoolcache/go/1.25.7/x64/pkg/tool/linux_amd64/link aw-mcpg/internal-o ache/go/1.25.7/x/tmp/go-build2320097585/b314/config.test docker insp�� 551622d230bb31fc-s by/bd0209c110101-w rtificates.crt",-buildmode=exe by/a501e7528e5b9bash e330148d0ce51d5e/usr/bin/runc (dns block)

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

@Claude Claude AI assigned Claude and lpcox Mar 13, 2026
@Claude Claude AI linked an issue Mar 13, 2026 that may be closed by this pull request
- Extract executeBackendToolCall helper to consolidate backend tool call execution logic
  - Reduces ~18 lines of duplication between guardBackendCaller.CallTool and callBackendTool
  - Unifies connection handling, request sending, error checking, and result unmarshaling

- Extract callSysServer helper to consolidate sysServer delegation logic
  - Reduces ~10 lines of duplication between sysInitHandler and sysListHandler
  - Centralizes tool marshaling and HandleRequest delegation

Fixes #1661 (parent issue), #1662 (backend call pattern), #1663 (sys server pattern)

Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
@Claude Claude AI changed the title [WIP] [duplicate-code] Fix duplicated code analysis issues in unified.go refactor: extract duplicate code patterns into helpers Mar 13, 2026
@Claude Claude AI requested a review from lpcox March 13, 2026 16:04
@lpcox lpcox marked this pull request as ready for review March 13, 2026 16:39
Copilot AI review requested due to automatic review settings March 13, 2026 16:39
@lpcox lpcox merged commit 8269bae into main Mar 13, 2026
12 checks passed
@lpcox lpcox deleted the claude/fix-duplicate-code-patterns branch March 13, 2026 16:39
Copy link
Contributor

Copilot AI left a comment

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/server/unified.go to remove structural duplication in two recurring request/dispatch patterns: sys-tool delegation and backend tools/call execution. This consolidates shared mechanics in helpers so future changes to these flows are less error-prone.

Changes:

  • Added callSysServer helper to centralize sys-tool tools/call request construction and delegation.
  • Added executeBackendToolCall helper to centralize backend connection acquisition, request dispatch, backend error checks, and result unmarshaling.
  • Updated sys handlers and DIFC backend-call sites to use the new helpers.

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

You can also share your feedback on Copilot code review. Take the survey.

Comment on lines +842 to +848
// Get or launch backend connection (use session-aware connection for stateful backends)
sessionID := g.ctx.Value(SessionIDContextKey)
if sessionID == nil {
sessionID = "default"
}

return executeBackendToolCall(g.ctx, g.server.launcher, g.serverID, sessionID.(string), toolName, args)
Comment on lines +843 to +848
sessionID := g.ctx.Value(SessionIDContextKey)
if sessionID == nil {
sessionID = "default"
}

return executeBackendToolCall(g.ctx, g.server.launcher, g.serverID, sessionID.(string), toolName, args)
Comment on lines +476 to +479
params, _ := json.Marshal(map[string]interface{}{
"name": toolName,
"arguments": map[string]interface{}{},
})
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 Analysis Report

3 participants