Skip to content

Conversation

@juruen
Copy link
Collaborator

@juruen juruen commented Mar 17, 2025

Context

This PR introduces an IOLogger struct that logs all stdin and stdout interactions between the client and the server.

This feature is particularly useful for debugging, as it provides visibility into the exact input and output exchanged between the client and the server.

By default, logging is disabled but can be enabled by setting the enable-command-logging flag to true. I considered enabling it only when the log level is set to debug, but decided that having a separate toggle would be preferable due to potential privacy concerns.

Copilot AI review requested due to automatic review settings March 17, 2025 12:38
Copy link
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

A PR to add an IOLogger for logging stdin and stdout between the client and server for debugging purposes.

  • Introduces an IOLogger struct with Read and Write methods that log data.
  • Adds tests in pkg/log/io_test.go to verify logged IO behavior.
  • Updates the server startup in cmd/server/main.go to conditionally wrap standard IO with the IOLogger based on a new command flag.

Reviewed Changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 1 comment.

File Description
pkg/log/io.go Introduces the IOLogger implementation and its logging behavior on Read and Write.
pkg/log/io_test.go Provides tests to verify logged input and output functionality.
cmd/server/main.go Modifies server initialization to retrieve and pass a command logging flag and binds a new flag.
Files not reviewed (1)
  • go.mod: Language not supported

Tip: Copilot only keeps its highest confidence comments to reduce noise and keep you focused. Learn more

@juruen juruen force-pushed the juruen/iologging branch from 2386301 to 6d63777 Compare March 17, 2025 12:49
Copy link
Collaborator

@SamMorrowDrums SamMorrowDrums left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can see how this would save a lot of pain when iterating on things.

@juruen juruen merged commit e0bffe9 into main Mar 18, 2025
3 checks passed
manian0430 pushed a commit to ChrisLally/github-mcp-server that referenced this pull request Apr 12, 2025
add iologger for debugging purposes
@gelu22
Copy link

gelu22 commented Sep 12, 2025

Progress Update - Phase 2 Completed ✅

🎯 Major Achievement: Endpoint Coverage Improvement

Successfully resolved missing API endpoints causing 404 errors:

Created new routes:

  • /api/export - PDF/CSV export endpoints
  • /api/tenant - Tenant management endpoints
  • /api/docker - Container information endpoints
  • /api/database - Database health and table info endpoints

📊 Test Results Analysis

Before (Phase 1): ~21% success rate (majority schema errors)
After (Phase 2): Significant improvement - 404 errors largely resolved

Current comprehensive test status:

  • ✅ Database schema issues: RESOLVED
  • ✅ Missing API endpoints: RESOLVED
  • 🔄 Authentication middleware errors: IN PROGRESS
  • 🔄 JWT validation issues: IN PROGRESS

🔍 Current Focus: Authentication & Authorization

Primary issues now:

  • 500 Internal Server Error responses from auth middleware
  • JWT token validation failures
  • Permission boundary violations
  • Session management inconsistencies

Pattern identified: Tests failing on authentication flow rather than missing endpoints - this is progress!

🚀 Next Steps - Phase 3: Authentication System

Immediate priorities:

  1. Debug authentication middleware - investigate 500 errors in auth flow
  2. Fix JWT validation - resolve token processing issues
  3. Improve RBAC implementation - correct role-based access control
  4. Session management - fix session lifecycle and timeout handling

Target: Achieve 80%+ comprehensive test success rate

💡 Key Insights

  • Systematic approach working: Phase 1 (schema) → Phase 2 (endpoints) → Phase 3 (auth)
  • Test failures now indicate functional system with auth issues vs broken infrastructure
  • Moving from "system broken" to "system needs configuration" - major progress

Status: Phase 2 Complete ✅ | Phase 3 Authentication Debugging In Progress 🔄

DaleSeo pushed a commit to DaleSeo/github-mcp-server that referenced this pull request Oct 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

4 participants