Skip to content

Collapse RPC logging split API and remove duplicated tag-branching#6829

Merged
lpcox merged 2 commits into
mainfrom
copilot/aw-parentmain001-fix-duplicate-code-pattern
Jun 1, 2026
Merged

Collapse RPC logging split API and remove duplicated tag-branching#6829
lpcox merged 2 commits into
mainfrom
copilot/aw-parentmain001-fix-duplicate-code-pattern

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Jun 1, 2026

RPC request/response logging exposed split APIs for with/without agent tags, which forced repeated shouldAttachTags branching in connection logging. This change collapses logging to a single request/response API with optional tag slices and removes duplicate branching at call sites.

  • Logger API consolidation (internal/logger/rpc_logger.go)

    • Reduced 4 public wrappers (LogRPC* + LogRPC*WithAgentSnapshot) to 2 public functions:
      • LogRPCRequest(..., agentSecrecy, agentIntegrity []string)
      • LogRPCResponse(..., err, agentSecrecy, agentIntegrity []string)
    • Kept tag attachment optional via nil slices.
  • Connection logging deduplication (internal/mcp/connection_logging.go, internal/mcp/connection.go)

    • Removed repeated if shouldAttachTags branches from outbound/inbound logging paths.
    • Introduced snapshot-to-tags extraction once and passed a nil-or-real snapshot through logging calls.
    • Simplified helper signatures by removing the boolean branch-control parameter where snapshot nilability already encodes intent.
  • Call-site and test alignment

    • Updated logger call sites and tests to use the unified API.
    • Renamed/adjusted test cases and comments that referenced removed split-wrapper functions.
// Before: branch at every call site
if shouldAttachTags {
    logger.LogRPCRequestWithAgentSnapshot(dir, serverID, method, payload, snapshot.Secrecy, snapshot.Integrity)
} else {
    logger.LogRPCRequest(dir, serverID, method, payload)
}

// After: single call, optional tags
secrecy, integrity := snapshotTags(snapshot) // nil,nil when no snapshot
logger.LogRPCRequest(dir, serverID, method, payload, secrecy, integrity)

Copilot AI changed the title [WIP] Fix duplicate code pattern in RPC logging API Collapse RPC logging split API and remove duplicated tag-branching Jun 1, 2026
Copilot finished work on behalf of lpcox June 1, 2026 03:54
Copilot AI requested a review from lpcox June 1, 2026 03:54
@lpcox lpcox marked this pull request as ready for review June 1, 2026 04:16
Copilot AI review requested due to automatic review settings June 1, 2026 04:16
Copy link
Copy Markdown
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

This PR simplifies RPC request/response logging by collapsing split “with/without agent tags” logger APIs into a single API that accepts optional tag slices, and then removes duplicated tag-attachment branching in MCP connection logging.

Changes:

  • Consolidated LogRPCRequest/LogRPCResponse into unified functions that accept optional agentSecrecy/agentIntegrity slices.
  • Removed repeated shouldAttach* branching at connection logging call sites by passing a nil-or-real *AgentTagsSnapshot and extracting tags once.
  • Updated MCP and logger tests/comments to align with the unified logging API.
Show a summary per file
File Description
internal/logger/rpc_logger.go Collapses RPC request/response logging APIs to accept optional agent tag slices.
internal/logger/rpc_logger_test.go Updates tests to call unified APIs; keeps coverage for tagged/untagged JSONL behavior.
internal/mcp/connection.go Deduplicates tag-attachment branching by selecting a nil-or-real logging snapshot once.
internal/mcp/connection_logging.go Introduces snapshotTags helper and removes duplicated logger branching in request/response logging helpers.
internal/mcp/connection_logging_test.go Updates helper calls to the new snapshot-based signature.
internal/mcp/sdk_method_dispatch_test.go Updates test comments to reflect unified optional-tag logging behavior.

Copilot's findings

Tip

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

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

@lpcox lpcox merged commit 319b187 into main Jun 1, 2026
29 checks passed
@lpcox lpcox deleted the copilot/aw-parentmain001-fix-duplicate-code-pattern branch June 1, 2026 04:23
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: RPC Logging Split API Pattern

3 participants