Skip to content

Conversation

@patrickcping
Copy link
Collaborator

Description

Fixes MCP protocol compliance violations in authentication wrapper code where fmt.Printf was causing protocol communication issues by writing directly to stdout/stderr. The MCP specification requires structured logging only.

Changes

  • internal/auth/client/wrapper.go: Replaced all fmt.Printf calls with structured logging using log.Info with proper key-value pairs
  • cmd/session/session.go: Fixed to use correct fmt.Printf for CLI output (CLI commands in cmd/ directory are exempt from MCP logging restrictions)
  • Removed format strings (\n), newlines, and trailing punctuation from log messages to follow MCP structured logging patterns

Why This Matters

The MCP specification requires that servers use structured logging only to stderr. Direct output to stdout/stderr via fmt.Printf, fmt.Println, print, or println breaks MCP protocol communication.

This fix ensures the MCP server's authentication flows properly use structured logging as required, preventing interference with protocol communication.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Testing

Test Results

All unit tests passed successfully:

make test
Test Output (All Passed)
=== RUN   TestRootCommand
--- PASS: TestRootCommand (0.00s)
=== RUN   TestLogoutCommand_FromRoot_Basic
--- PASS: TestLogoutCommand_FromRoot_Basic (0.00s)
=== RUN   TestLogoutCommand_Direct_Success
--- PASS: TestLogoutCommand_Direct_Success (0.00s)
=== RUN   TestRunCommand_FromRoot_NoServerRun
--- PASS: TestRunCommand_FromRoot_NoServerRun (0.00s)
=== RUN   TestSessionCommand_FromRoot_Basic
--- PASS: TestSessionCommand_FromRoot_Basic (0.00s)
PASS
ok      github.com/pingidentity/pingone-mcp-server/cmd  (cached)
...
All tests passed

Manual Validation

  • ✅ Authentication flows use structured logging
  • ✅ No fmt.Printf in MCP server code (internal/ packages)
  • ✅ CLI commands still output to users correctly
  • ✅ MCP protocol communication not interfered with

Checklist

  • Code follows MCP specification (especially logging requirements)
  • make test passes
  • Error handling follows project patterns
  • All code uses structured logging (no fmt.Printf in internal/)
  • Tests cover functionality
  • PR template is filled out completely

Related Issues

Fixes authentication flow logging issues that caused warnings in MCP clients due to protocol violations.

Additional Context

This is part 1 of a 2-part change. Part 2 (separate PR) will add automated enforcement mechanisms (golangci-lint configuration, Makefile checks, and documentation) to prevent future violations.

Fixes MCP protocol compliance violations in authentication wrapper code where fmt.Printf was causing protocol communication issues.

Changes:
- Replaced all fmt.Printf calls in internal/auth/client/wrapper.go with structured logging using log.Info
- Fixed cmd/session/session.go to use correct fmt.Printf for CLI output (CLI commands are exempt from MCP logging restrictions)
- Removed format strings, newlines, and trailing punctuation from log messages to follow MCP structured logging patterns

This ensures the MCP server's authentication flows properly use structured logging as required by the MCP specification, preventing interference with protocol communication on stdout/stderr.
@henryrecker-pingidentity henryrecker-pingidentity merged commit 348e7ad into main Dec 12, 2025
9 checks passed
@henryrecker-pingidentity henryrecker-pingidentity deleted the fix/auth-wrapper-logging-mcp-protocol/pc/20251212 branch December 12, 2025 15:34
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.

3 participants