Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Jan 18, 2026

Semantic analysis identified duplicate function implementations across authentication, sanitization, and logging packages—3 sanitization functions with 80-90% overlap and 2 auth parsers sharing 70% of logic.

Changes

Sanitization consolidation (internal/logger/sanitize)

  • Added TruncateSecret(string) and TruncateSecretMap(map[string]string) for prefix truncation (first 4 chars + "...")
  • Removed duplicate implementations from auth/header.go and launcher/launcher.go
  • Single source of truth for all secret sanitization

Auth parsing consolidation (internal/auth)

  • Added ExtractAgentID(authHeader) convenience wrapper around ParseAuthHeader()
  • Updated guard.ExtractAgentIDFromAuthHeader() to delegate to auth package
  • Deprecated guard function marked for future removal

Reduced rpc_logger.go complexity

  • Split 314-line file into focused modules:
    • rpc_logger.go (108 lines): coordination and public API
    • rpc_formatter.go (129 lines): text/markdown formatting
    • rpc_helpers.go (82 lines): payload processing utilities
  • No API changes, tests unchanged

Documentation

  • Added package-level docs clarifying responsibilities for auth, guard, sanitize, and logger RPC modules
  • Renamed global_patterns.goglobal_helpers.go to reflect actual purpose

Example

Before (duplicated across 3 files):

// auth/header.go
func sanitizeForLogging(input string) string {
    if len(input) > 4 { return input[:4] + "..." }
    // ...
}

// launcher/launcher.go  
func sanitizeEnvForLogging(env map[string]string) map[string]string {
    // same logic for maps
}

After (centralized):

// logger/sanitize/sanitize.go
sanitize.TruncateSecret(authHeader)
sanitize.TruncateSecretMap(envVars)

All changes are internal refactoring with complete backward compatibility.

Original prompt

This section details on the original issue you should resolve

<issue_title>[refactor] Semantic Function Clustering Analysis - Refactoring Opportunities</issue_title>
<issue_description>## Executive Summary

Analysis of 44 non-test Go files in internal/ identified 8 high-priority refactoring opportunities across authentication, sanitization, logging, and validation domains. The most significant findings include duplicated sanitization logic (3 implementations), auth header parsing duplication (2 packages), and complexity in the logger package (10 files, 65 unexported helpers).

Estimated Total Effort: 4-6 hours of focused refactoring across 3 phases.


Files Analyzed (30 files)

Package Files Non-Test LOC Key Areas
auth/ 1 ~100 Authentication header parsing
config/ 5 ~800 Configuration validation & parsing
guard/ 4 ~300 Security context & agent ID extraction
launcher/ 1 ~400 Process management & env sanitization
logger/ 10 ~1200 Multi-format logging infrastructure
server/ 6 ~500 HTTP server & middleware
mcp/ 3 ~250 MCP protocol types

🔴 Critical Findings: Duplicate Functions

1. Sanitization Logic Duplication (HIGH PRIORITY)

Problem: 3 different implementations of "first 4 chars + ..." sanitization across packages.

Duplicate Functions:

// internal/auth/header.go:35
func sanitizeForLogging(input string) string {
    if len(input) > 4 {
        return input[:4] + "..."
    } else if len(input) > 0 {
        return "..."
    }
    return ""
}

// internal/launcher/launcher.go:22
func sanitizeEnvForLogging(env map[string]string) map[string]string {
    // ... same 4-char logic for map values
    sanitized[key] = value[:4] + "..."
}

// internal/logger/rpc_logger.go:50 (uses internal/logger/sanitize package)
func truncateAndSanitize(payload string, maxLength int) string {
    sanitized := sanitize.SanitizeString(payload)  // Uses regex patterns
    // ... then truncates
}

Similarity: 80-90% functional overlap (prefix truncation for logging safety)

Impact:

  • Code maintainability: Changes to sanitization logic need 3 updates
  • Inconsistency risk: Different truncation lengths (4 chars vs none)
  • Testing burden: 3 sets of tests for same behavior

Recommendation:

  1. Move to internal/logger/sanitize package
  2. Create unified API:
    // sanitize.TruncateSecret(s string) string          // 4 chars + "..."
    // sanitize.TruncateSecretMap(m map[string]string)   // Apply to all values
  3. Update 3 call sites: auth/header.go, launcher/launcher.go, rpc_logger.go

Effort: 1 hour (30 min refactor + 30 min test updates)


2. Auth Header Parsing Duplication (MEDIUM PRIORITY)

Problem: Auth header parsing logic exists in 2 packages with 70% code overlap.

Duplicate Functions:

// internal/auth/header.go:56
func ParseAuthHeader(authHeader string) (apiKey, agentID string, error error) {
    // Handles: Bearer, Agent, plain API key
    // Returns: apiKey + agentID + error
    // Has logging: log.Printf(...)
}

// internal/guard/context.go:42
func ExtractAgentIDFromAuthHeader(authHeader string) string {
    // Handles: Bearer, Agent, plain
    // Returns: only agentID (no error)
    // No logging
}

Code Overlap: 70% (both parse Bearer/Agent prefixes)

Differences:

  • ParseAuthHeader: Full error handling, logging, returns tuple
  • ExtractAgentIDFromAuthHeader: Silent fallback to "default", no errors

Impact:

  • Bug risk: If auth logic changes, must update both
  • Currently: server/auth.go uses neither (direct header comparison)
  • Inconsistency: Guard package should delegate to auth package

Recommendation:

  1. Make internal/auth the single source of truth
  2. Add convenience method to auth package:
    // auth.ExtractAgentID(header string) string  // Wraps ParseAuthHeader, no error
  3. Update guard/context.go to call auth.ExtractAgentID()
  4. Deprecate guard.ExtractAgentIDFromAuthHeader with comment

Effort: 45 minutes (code + tests + validation)


3. Error Message Extraction Confusion (LOW PRIORITY)

Problem: File naming suggests duplication but files serve different purposes.

Files:

  • internal/logger/error_formatting.go (47 lines) - Single function ExtractErrorMessage
  • internal/logger/global_patterns.go (83 lines) - Global state helpers (6 functions)

Analysis:

  • INCORRECT initial assumption: These are NOT duplicates
  • error_formatting.go: Log line cleanup (removes timestamps/levels)
  • global_patterns.go: Mutex-wrapped global logger init/close helpers
  • Misleading name: global_patterns.go should be renamed to global_helpers.go or global_state.go

Recommendation:

  1. Rename global_patterns.goglobal_helpers.go (better reflects purpose)
  2. No co...

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI and others added 6 commits January 18, 2026 17:11
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Copilot AI changed the title [WIP] Refactor semantic function clustering analysis for improvements Consolidate duplicate sanitization and auth parsing logic Jan 18, 2026
Copilot AI requested a review from lpcox January 18, 2026 17:22
@lpcox lpcox marked this pull request as ready for review January 18, 2026 17:23
@lpcox lpcox merged commit 5e9b13e into main Jan 18, 2026
3 checks passed
@lpcox lpcox deleted the copilot/refactor-semantic-function-clustering branch January 18, 2026 17: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.

[refactor] Semantic Function Clustering Analysis - Refactoring Opportunities

2 participants