Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Jan 16, 2026

The codebase had two deprecated wrapper functions (sanitizePayload and sanitizeSecrets) that added indirection without providing value, flagged by semantic function clustering analysis.

Changes

  • Removed wrapper functions: Deleted sanitizePayload() from jsonl_logger.go and sanitizeSecrets() from markdown_logger.go (15 lines total)
  • Updated call sites: Direct calls to sanitize.SanitizeJSON() and sanitize.SanitizeString() throughout logger code and tests
  • Preserved auth sanitization: Kept auth/header.go:sanitizeForLogging() - implements distinct prefix-based logic for auth header previews

Before/After

// Before: Indirect call through wrapper
entry.Payload = sanitizePayload(payloadBytes)

// After: Direct call to centralized package
entry.Payload = sanitize.SanitizeJSON(payloadBytes)

The sanitize package now serves as the single source of truth for regex-based sanitization, with auth-specific sanitization remaining separate by design.

Original prompt

This section details on the original issue you should resolve

<issue_title>[refactor] Semantic Function Clustering Analysis - January 2026 Update</issue_title>
<issue_description>## Overview

Fresh analysis of 43 non-test Go files (~7,830 lines, 299 functions) in the MCP Gateway codebase reveals that significant refactoring progress has been made since previous analyses. The codebase is now well-organized with the newly created internal/auth/ package consolidating authentication logic, and the internal/logger/sanitize/ package providing centralized sanitization.

Key Findings:

  • Auth package created - internal/auth/ now provides centralized authentication parsing
  • Sanitization consolidated - internal/logger/sanitize/ package with unified API
  • ⚠️ 3 sanitization variations remain - Different strategies for different purposes
  • ⚠️ 2 deprecated wrapper functions - Kept for backward compatibility
  • Excellent organization - 9/11 packages follow single-responsibility patterns

Comparison to Previous Analyses:


Full Report

Analysis Metadata

  • Repository: githubnext/gh-aw-mcpg
  • Total Go Files Analyzed: 43 (excluding tests)
  • Total Lines of Code: 7,830
  • Total Functions: 299
  • Packages Analyzed: 13
  • Detection Method: Pattern analysis + semantic code review
  • Analysis Date: 2026-01-15
  • Workflow Run: §21033130477

Package Organization Assessment

✅ Excellent Organization (9 packages)

Package Files Functions Lines Purpose Status
auth/ 1 3 99 Auth header parsing NEW! Well-designed
cmd/ 2 9 ~450 CLI commands (Cobra) ✅ Excellent
difc/ 5 66 ~1,240 DIFC security labels ✅ Excellent
guard/ 4 19 ~340 Security guards ✅ Excellent
launcher/ 1 5 226 Backend process mgmt ✅ Excellent
mcp/ 2 23 ~940 MCP protocol types ✅ Excellent
sys/ 1 6 112 System utilities ✅ Excellent
timeutil/ 1 1 27 Time formatting ✅ Excellent
tty/ 2 3 53 TTY detection ✅ Excellent

✅ Good Organization (2 packages)

Package Files Functions Lines Purpose Status
config/ 5 37 ~1,034 Config parsing/validation ✅ Good (well-separated)
testutil/ 4 22 ~440 Test utilities ✅ Good

⚠️ Minor Optimization Opportunities (2 packages)

Package Files Functions Lines Issue Priority
logger/ 9 57 ~1,528 Deprecated wrappers + variations Low
server/ 6 48 ~1,502 Minor duplication in setup Low

Progress Since Previous Analyses

✅ Completed Refactoring (Issues #196, #226, #256)

1. Auth Package Created ✅

Previous Issue: Auth logic was split between server/auth.go and guard/context.go, creating potential inconsistencies.

Resolution: Created internal/auth/ package with:

// internal/auth/header.go
func ParseAuthHeader(authHeader string) (apiKey, agentID string, error error)
func ValidateAPIKey(provided, expected string) bool
func sanitizeForLogging(input string) string  // internal helper

Impact:

  • ✅ Single source of truth for auth header parsing
  • ✅ MCP spec 7.1 compliant (plain API key, not Bearer)
  • ✅ Backward compatibility (supports Bearer and Agent formats)
  • ✅ Both server/auth.go and guard/context.go now reference this package

Files Updated:

  • internal/auth/header.go - Created with comprehensive parsing logic
  • internal/server/auth.go - Comments reference auth package
  • internal/guard/context.go - Comments reference auth package

2. Sanitization Package Created ✅

Previous Issue: Sanitization functions scattered across multiple packages (launcher, logger files).

Resolution: Created internal/logger/sanitize/ package with:

// internal/logger/sanitize/sanitize.go
func SanitizeString(message string) string
func SanitizeJSON(payloadBytes []byte) json.RawMessage

Features:

  • 10 comprehensive regex patterns for secrets (tokens, keys, passwords, JWTs)
  • GitHub PAT patterns (ghp_, github_pat_)
  • Bearer token detection
  • JSON field-value pair sanitization
  • Automatic [REDACTED] replacement

**Files Using Sanit...


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

- Replaced sanitizePayload() with direct calls to sanitize.SanitizeJSON()
- Replaced sanitizeSecrets() with direct calls to sanitize.SanitizeString()
- Updated test files to use centralized sanitize package directly
- All tests passing, build successful

Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Copilot AI changed the title [WIP] Refactor Semantic Function Clustering analysis for January 2026 Remove deprecated sanitization wrapper functions Jan 16, 2026
Copilot AI requested a review from lpcox January 16, 2026 06:09
@lpcox
Copy link
Collaborator

lpcox commented Jan 16, 2026

@copilot please fix this linting error

run golangci-lint
Running [/home/runner/golangci-lint-2.8.0-linux-amd64/golangci-lint run --timeout=5m] in [/home/runner/work/gh-aw-mcpg/gh-aw-mcpg] ...
Error: internal/sys/sys_test.go:46:7: S1009: should omit nil check; len() for nil slices is defined as zero (staticcheck)
if tt.serverIDs != nil && len(tt.serverIDs) > 0 {
^
1 issues:

  • staticcheck: 1

Error: issues found
Ran golangci-lint in 2831ms

@lpcox lpcox marked this pull request as ready for review January 16, 2026 06:17
@lpcox lpcox merged commit b60c6d4 into main Jan 16, 2026
2 of 4 checks passed
@lpcox lpcox deleted the copilot/refactor-semantic-function-clustering branch January 16, 2026 06:17
Copilot stopped work on behalf of lpcox due to an error January 16, 2026 06:17
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 - January 2026 Update

2 participants