[duplicate-code] Duplicate Code Pattern: Repeated shouldAttachAgentTags Conditional Logging in connection.go

[github-actions]

Part of duplicate code analysis: #3827

Summary

The same 5-line conditional logging block (if shouldAttachAgentTags { ... } else { ... }) for RPC responses is copy-pasted 3 times within SendRequestWithServerID in internal/mcp/connection.go. Each copy dispatches to either LogRPCResponseWithAgentSnapshot or LogRPCResponse based on the same boolean flag.

Duplication Details

Pattern: if shouldAttachAgentTags response logging

• Severity: Medium

• Occurrences: 3 identical blocks

• Locations:

  • internal/mcp/connection.go (lines 457–461) — inside if c.httpTransportType == HTTPTransportPlainJSON
  • internal/mcp/connection.go (lines 472–476) — inside SDK streamable/SSE branch
  • internal/mcp/connection.go (lines 488–492) — stdio branch

• Code Sample (repeated verbatim three times):

  if shouldAttachAgentTags {
      logger.LogRPCResponseWithAgentSnapshot(logger.RPCDirectionInbound, serverID, responsePayload, err, snapshot.Secrecy, snapshot.Integrity)
  } else {
      logger.LogRPCResponse(logger.RPCDirectionInbound, serverID, responsePayload, err)
  }

There is also a similar block for the outbound request (lines 438–442), but it only appears once and is less problematic.

Impact Analysis

• Maintainability: If the logging signature changes (e.g., adding a new tag type or changing the direction argument), all three copies must be updated in sync. A single missed update creates silent inconsistency.
• Bug Risk: Medium — the pattern is correct today, but divergence becomes likely under future refactoring.
• Code Bloat: ~15 lines of identical code in one function.

Refactoring Recommendations

1. Extract a logRPCResponse helper function

   Add a small unexported helper in internal/mcp/connection.go (or a nearby log_helpers.go):

   // logRPCResponse logs an inbound RPC response, optionally attaching agent DIFC tag snapshots.
   func logRPCResponse(serverID string, payload []byte, err error, shouldAttachTags bool, snapshot agentTagsSnapshot) {
       if shouldAttachTags {
           logger.LogRPCResponseWithAgentSnapshot(logger.RPCDirectionInbound, serverID, payload, err, snapshot.Secrecy, snapshot.Integrity)
       } else {
           logger.LogRPCResponse(logger.RPCDirectionInbound, serverID, payload, err)
       }
   }

   Then each of the three call sites becomes a single line:

   logRPCResponse(serverID, responsePayload, err, shouldAttachAgentTags, snapshot)

   Estimated effort: ~30 minutes. No functional change — pure refactor.

2. Alternative: extend LogRPCResponse to accept optional tags

   Add a variadic agentTags ...[]string parameter to LogRPCResponse so callers don't need to branch. This requires a change to the public logger API but eliminates the if/else entirely.

Implementation Checklist

• Review the three duplicate blocks in SendRequestWithServerID
• Extract logRPCResponse helper (or equivalent)
• Replace the three if/else blocks with single helper calls
• Confirm unit tests still pass (make test-unit)

Parent Issue

See parent analysis report: #3827
Related to #3827

Generated by Duplicate Code Detector · ● 1.7M · ◷

• expires on Apr 22, 2026, 6:13 AM UTC

[github-actions]

🤖 This is an automated response from Repo Assist.

This is a straightforward refactor with clear value — I've implemented it in PR #3840.

The fix extracts a logInboundRPCResponse helper function in connection.go and replaces the three identical if shouldAttachAgentTags { ... } else { ... } blocks with single-line calls. The helper signature is:

func logInboundRPCResponse(serverID string, payload []byte, err error, shouldAttachTags bool, snapshot *AgentTagsSnapshot)

When shouldAttachTags is true, snapshot is guaranteed non-nil by the caller's shouldAttachAgentTags := hasSnapshot && difc.IsSinkServerID(serverID) guard. No functional change — pure refactor.

Warning

⚠️ Firewall blocked 6 domains

The following domains were blocked by the firewall during workflow execution:

• go.opentelemetry.io
• go.yaml.in
• golang.org
• google.golang.org
• gopkg.in
• proxy.golang.org

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "go.opentelemetry.io"
    - "go.yaml.in"
    - "golang.org"
    - "google.golang.org"
    - "gopkg.in"
    - "proxy.golang.org"

See Network Configuration for more information.

Generated by Repo Assist · ● 5M · ◷

To install this agentic workflow, run

gh aw add githubnext/agentics/workflows/repo-assist.md@851905c06e905bf362a9f6cc54f912e3df747d55