-
Notifications
You must be signed in to change notification settings - Fork 15
Closed
Description
Part of duplicate code analysis: #1661
Summary
Two sys-tool handler closures in internal/server/unified.go (sysInitHandler and sysListHandler, registered by registerSysTools) share an identical 10-line block for delegating a call to us.sysServer. Both handlers: marshal a tools/call params object, invoke HandleRequest, check the error, and return nil, result, nil. The block differs only in the tool name string.
Duplication Details
Pattern: marshal params → sysServer.HandleRequest("tools/call") → check error → return result
- Severity: Medium
- Occurrences: 2
- Locations:
internal/server/unified.golines 478–494 (insidesysInitHandler)internal/server/unified.golines 519–534 (insidesysListHandler)
Block in sysInitHandler (lines 478–494):
params, _ := json.Marshal(map[string]interface{}{
"name": "sys_init",
"arguments": map[string]interface{}{},
})
result, err := us.sysServer.HandleRequest("tools/call", json.RawMessage(params))
if err != nil {
logger.LogError("client", "MCP session initialization: sys_init call failed, session=%s, error=%v", sessionID, err)
return newErrorCallToolResult(err)
}
resultJSON, _ := json.Marshal(result)
sanitizedResult := sanitize.SanitizeString(string(resultJSON))
logger.LogInfo("client", "MCP session initialization complete, session=%s, result=%s", sessionID, sanitizedResult)
return nil, result, nilNearly identical block in sysListHandler (lines 519–534):
params, _ := json.Marshal(map[string]interface{}{
"name": "sys_list_servers",
"arguments": map[string]interface{}{},
})
result, err := us.sysServer.HandleRequest("tools/call", json.RawMessage(params))
if err != nil {
logger.LogError("client", "MCP sys_list_servers error, session=%s, error=%v", sessionID, err)
return newErrorCallToolResult(err)
}
resultJSON, _ := json.Marshal(result)
logger.LogInfo("client", "MCP sys_list_servers response, session=%s, result=%s", sessionID, string(resultJSON))
return nil, result, nilThe structural differences are:
- The tool name string (
"sys_init"vs"sys_list_servers") - The log message text (different per tool)
Impact Analysis
- Maintainability: Adding a third sys tool (e.g.,
sys_reload) will require copy-pasting this block again. - Bug Risk: Low currently, but any change to how sysServer results are logged or returned must be replicated.
- Code Bloat: ~10 lines per handler — small but the pattern will grow as more sys tools are added.
Refactoring Recommendations
-
Extract
callSysServerhelper- Location:
internal/server/unified.go - Signature:
func (us *UnifiedServer) callSysServer(sessionID, toolName string) (*sdk.CallToolResult, interface{}, error) - Handles: marshal → HandleRequest → check error → log → return
- Each handler only provides the tool name and its unique pre-call logic.
- Estimated effort: ~30 minutes, low risk.
- Location:
-
Alternative: direct method call without JSON round-trip
- The
json.Marshal → HandleRequest(json.RawMessage)pattern serializes and immediately deserializes. Consider callingus.sysServer.callTool(name, args)directly if theSysServertype can expose it.
- The
Implementation Checklist
- Review duplication findings
- Extract
callSysServer(sessionID, toolName string)helper onUnifiedServer - Update
sysInitHandlerto use helper - Update
sysListHandlerto use helper - Verify future
sys_*tools follow the helper pattern - Update tests
Parent Issue
See parent analysis report: #1661
Related to #1661
Generated by Duplicate Code Detector
- expires on Mar 15, 2026, 3:01 AM UTC
Reactions are currently unavailable
Metadata
Metadata
Type
Fields
Give feedbackNo fields configured for issues without a type.