Enhance configuration management and signal handling in mcpproxy #2
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
Dumbris
commented
Jun 24, 2025
Dumbris
commented
- Added automatic creation of default configuration file and directory on first-time setup.
- Improved configuration loading priority with fallback mechanisms.
- Implemented robust signal handling for graceful shutdown, including context-aware operations and logging.
- Updated server background operations to respect cancellation context for better resource management.
- Added automatic creation of default configuration file and directory on first-time setup. - Improved configuration loading priority with fallback mechanisms. - Implemented robust signal handling for graceful shutdown, including context-aware operations and logging. - Updated server background operations to respect cancellation context for better resource management.
- Renamed CacheRecord to Record and CacheStats to Stats for clarity. - Updated related methods and types to reflect the new naming conventions. - Improved error handling in various functions by deferring error checks. - Enhanced logging synchronization to ensure proper resource management.
- Updated function signatures in main.go and manager.go for clarity by removing unused parameters. - Enhanced error handling in mcp.go and client.go by ensuring proper return values. - Introduced constants for operation names in mcp.go to improve readability and maintainability. - Removed deprecated test fixtures in fixtures_test.go to streamline the codebase.
- Introduced a constant for the default port in config.go to enhance maintainability. - Updated DefaultConfig to use the new constant for setting the Listen address. - Improved error handling in loader.go by changing file write permissions for security. - Added a testServerConnection method in e2e_test.go to verify server readiness more effectively. - Refactored function signatures in various files for clarity and consistency.
- Updated .goreleaser.yaml to define builds for Linux, Windows, and macOS with appropriate settings. - Added Windows Testing Guide to README.md for improved clarity on running tests in Windows environments. - Modified GitHub Actions workflows to support platform-specific test execution and binary builds. - Introduced scripts for running unit tests on Windows with proper argument handling. - Refactored main.go to support tray functionality conditionally based on build tags.
- Removed redundant CGO_ENABLED environment variable from jobs in e2e-tests.yml. - Added CGO_ENABLED=0 to build commands in unit-tests.yml for cross-compilation consistency. - Enhanced logging in manager.go by changing warning to debug level for disconnected clients. - Optimized GetTotalToolCount method to minimize network calls during shutdown.
- Updated e2e-tests.yml to run mcpproxy in the background for log file creation. - Enhanced e2e_test.go to count log messages more reliably and verify goroutine presence. - Adjusted assertions to allow for tolerance in concurrent logging scenarios.
- Updated e2e-tests.yml to build mcpproxy binary for both Windows and Unix environments. - Modified TestE2E_MCPProxyWithLogging to dynamically determine binary name based on OS. - Improved TestE2E_ConcurrentLogging to use unique log filenames and adjusted concurrency settings for Windows.
- Updated e2e-tests.yml to store the mcpproxy process in a variable before stopping it, improving clarity and maintainability of the workflow.
- Changed log path to use the LOCALAPPDATA environment variable for better compatibility. - Updated compliance check to match specific OS standard compliance information.
- Changed log path output to use the LOCALAPPDATA environment variable. - Updated OS standard message to reflect Windows Application Data Guidelines.
…ling - Updated PowerShell script to include try-catch for mcpproxy process management. - Reorganized log directory and file existence checks for clarity and reliability. - Enhanced log content verification to ensure compliance with OS standards.
- Updated background initialization methods to use read locks for accessing server context. - Added cancellation of the old context in StartServer to avoid potential race conditions. - Improved tool re-indexing logic after configuration reload to ensure thread safety.
…Documentation - Introduced COMPATIBILITY_SUMMARY.md detailing schema changes for `upstream_servers` and `call_tool` tools, transitioning to JSON string parameters for improved compatibility. - Created GEMINI_COMPATIBILITY.md to address known issues with Gemini 2.5 Pro Preview 06-05 and provide updated usage examples and recommendations. - Refactored mcp.go to support new JSON string parameters while maintaining backward compatibility with legacy object formats.
…N format - Modified COMPATIBILITY_SUMMARY.md to include changes for `env`, `headers`, and `patch` parameters to JSON strings. - Updated GEMINI_COMPATIBILITY.md with examples reflecting the new JSON string format for `upstream_servers`. - Refactored mcp.go to handle new JSON string parameters while ensuring backward compatibility with legacy formats.
Dumbris
added a commit
that referenced
this pull request
Nov 2, 2025
Implements Issues #1, #2, #4, and #11 (Layers 1, 3, 4) from docker-recovery-improvements.md **Issue #11: Duplicate Container Spawning Prevention** - Layer 1 (Idempotent Creation): ensureNoExistingContainers() cleans up all existing containers before creating new ones - Layer 3 (Distributed Locks): Per-server mutex prevents concurrent container creation races - Layer 4 (Enhanced Health Verification): verifyContainerHealthy() checks Docker container state AND responsiveness **Issue #1: Docker-only Filtering** - ForceReconnectAll() now filters to only reconnect Docker-based servers - HTTP/SSE/stdio servers are skipped, preventing unnecessary reconnections **Issue #2: Container Health Verification** - verifyContainerHealthy() detects frozen containers when Docker paused - Uses docker inspect to verify container running + responsive - Prevents skipping reconnection for zombie containers **Issue #4: Add Recovering State** - New StateCoreRecoveringDocker state provides UX feedback during recovery - EventDockerRecovered triggers transition from error → recovering → launching - Tray menu shows "Docker engine recovered - reconnecting servers..." **Files Modified:** - internal/upstream/core/docker.go: ensureNoExistingContainers(), GetContainerID() - internal/upstream/core/connection.go: container lock acquisition, pre-cleanup - internal/upstream/core/container_lock.go: NEW - distributed lock implementation - internal/upstream/core/client.go: GetContainerID() accessor - internal/upstream/manager.go: verifyContainerHealthy(), Docker-only filtering - internal/upstream/managed/client.go: GetContainerID() delegation - cmd/mcpproxy-tray/internal/state/states.go: StateCoreRecoveringDocker, EventDockerRecovered - cmd/mcpproxy-tray/main.go: state mapping, handleDockerRecovering() - internal/tray/connection_state.go: ConnectionStateRecoveringDocker **Testing:** - All upstream package tests passing (./internal/upstream/...) - Code compiles cleanly (go build ./...) - State machine transitions validated **Impact:** - Prevents duplicate container spawning during concurrent reconnections - Eliminates resource exhaustion from orphaned containers - Better UX - users see recovery in progress instead of errors - Only Docker servers affected by Docker recovery (not HTTP/SSE)
Dumbris
added a commit
that referenced
this pull request
Nov 3, 2025
Added SHUTDOWN_BUG_ANALYSIS.md documenting three critical shutdown bugs: **Bug #1 - Core SIGTERM Hang (FIXED ✅)** - Issue: Core process hangs indefinitely on SIGTERM, never exits - Root Cause: Deadlock in parallel disconnect - servers in "Connecting" state hold write locks, blocking disconnect goroutines - Fix: Added 5-second timeout to force-proceed to Docker cleanup - Result: Shutdown completes in 9s with 100% container cleanup - Files: internal/upstream/manager.go (lines 419-450) **Bug #2 - Tray Quit Doesn't Stop Core (PARTIALLY FIXED⚠️ )** - Original Issue: processMonitor nil, silent skip of termination logic - Fix Applied: Added ownership tracking, PID discovery via /api/v1/status - New Issue: cmd.Wait() race condition causes tray to hang - Core terminates successfully ✅, but tray hangs indefinitely ❌ - Files: cmd/mcpproxy-tray/main.go, internal/server/server.go **Bug #2.1 - cmd.Wait() Race (NEW ❌)** - Both monitor() goroutine and Stop() method call cmd.Wait() - Second call blocks forever (can only call Wait() once per process) - Recommended Fix: Use event channel instead of cmd.Wait() - Files: cmd/mcpproxy-tray/internal/monitor/process.go Document includes: - Detailed root cause analysis with code examples - Timeline of events and evidence from logs - Test results showing what works and what doesn't - Recommended fixes with implementation code - Comprehensive testing recommendations 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Dumbris
added a commit
that referenced
this pull request
Nov 3, 2025
This commit fixes two critical bugs in the tray application's shutdown sequence that prevented proper Docker container cleanup when quitting the tray. Bug #2.1: cmd.Wait() Race Condition - SYMPTOM: Tray hung indefinitely (20+ seconds) after sending SIGTERM to core - ROOT CAUSE: Both monitor() goroutine and Stop() method called cmd.Wait() on same process Since cmd.Wait() can only be called once, the second call blocked forever - FIX: Added doneCh channel that closes when monitor() exits. Stop() now waits on doneCh instead of calling cmd.Wait() again - FILES: cmd/mcpproxy-tray/internal/monitor/process.go:84, 112, 217-242, 300, 312 Bug #2.2: Context Cancellation Sends SIGKILL Before SIGTERM - SYMPTOM: Core received SIGKILL instead of SIGTERM, never ran shutdown sequence, left 1/7 containers orphaned (everything-server) - ROOT CAUSE: ProcessMonitor.Shutdown() called pm.cancel() BEFORE pm.Stop() Cancelling context for exec.CommandContext() causes Go to send SIGKILL immediately - FIX: Moved pm.cancel() to AFTER pm.Stop() completes, allowing graceful SIGTERM - FILES: cmd/mcpproxy-tray/internal/monitor/process.go:283-307 Additional Improvements: - Changed syscall.Kill(-pid, SIGTERM) to syscall.Kill(pid, SIGTERM) for direct signal delivery - Works correctly with shell wrapper (zsh -l -c "exec mcpproxy") - Added comprehensive documentation in SHUTDOWN_BUG_ANALYSIS.md Test Results (AFTER FIX): ✅ Tray exits in 12 seconds (allows time for container cleanup) ✅ Core receives SIGTERM and runs full shutdown sequence ✅ All 7/7 Docker containers cleaned up successfully ✅ No cmd.Wait() race condition ✅ No orphaned containers 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Dumbris
added a commit
that referenced
this pull request
Nov 14, 2025
Related #2-windows-installer Implement comprehensive Windows installer for MCPProxy with dual-framework support (Inno Setup and WiX Toolset) for flexible deployment options. ## Changes ### Installer Implementation - Add Inno Setup installer script (scripts/installer.iss) with multi-arch support - Add WiX installer definitions for amd64 and arm64 (wix/Package.wxs, wix/Package-arm64.wxs) - Add PowerShell build script (scripts/build-windows-installer.ps1) for local builds - Configure system PATH modification for CLI access - Add Start Menu shortcuts for tray application - Implement in-place upgrade logic preserving user data - Add post-install launch option with silent mode support ### CI/CD Integration - Update release.yml workflow with Windows installer build steps - Update prerelease.yml workflow with Windows installer build steps - Add Inno Setup installation via Chocolatey in workflows - Add Windows installer artifacts to release assets - Update release notes template with Windows installation instructions ### Bug Fixes - Fix Windows named pipe path doubling issue in tray dialer (dialer.go) - Fix CREATE_NO_WINDOW flag handling in process monitor - Reorder condition checks to prevent fallback logic errors ### Documentation - Update CLAUDE.md with Windows installer build instructions - Update README.md with Windows installation section - Add comprehensive specification in specs/002-windows-installer/ - Create quickstart guide for local testing and development ## Features - Multi-architecture support (amd64 and arm64) - Dual installer framework (Inno Setup + WiX Toolset) - Automatic PATH configuration - Start Menu shortcuts - In-place upgrades with data preservation - Silent installation mode - Windows 10 version 21H2+ and Windows 11 support ## Testing Tested on Windows 11 ARM64 with Parallels Desktop: - Installer builds successfully for both architectures - PATH configuration works correctly - Tray application launches from Start Menu - Named pipe communication fixed and verified
17 tasks
technicalpickles
added a commit
to technicalpickles/mcpproxy-go
that referenced
this pull request
Dec 1, 2025
Enhanced docs/oauth-zero-config.md with detailed documentation for: 1. Server States Section: - Connected states: ready (authenticated), connected (no token) - Waiting states: pending_auth (normal waiting state, not an error) - Transitional states: connecting, authenticating - Error states: disconnected, error 2. Checking Authentication Status: - How to use `mcpproxy auth status` command - Example output with emoji indicators (✅⏳❌) - Status meaning explanations 3. Troubleshooting Section with 4 Common Issues: Issue smart-mcp-proxy#1: Server Shows "Pending Auth" State - Symptoms: ⏳ icon in tray, pending_auth status - Cause: OAuth detected but user hasn't authenticated - Solution: Use `auth login` or tray "Authenticate" action - Clarification: NOT an error, intentional deferral Issue smart-mcp-proxy#2: Authentication Token Expired - Symptoms: Was working, now shows "Auth Failed" - Cause: OAuth token expired (1-24 hour lifetime) - Solution: Re-authenticate with `auth login` Issue smart-mcp-proxy#3: OAuth Detection Not Working - Symptoms: No pending_auth, just connection failures - Diagnosis: Check doctor output, logs, manual OAuth test - Common causes: Non-standard endpoints, firewall issues - Solution: Add explicit OAuth configuration Issue smart-mcp-proxy#4: OAuth Login Opens Browser But Fails - Symptoms: Browser opens, approval given, but still fails - Diagnosis: Check callback logs for authorization code - Common causes: Port conflict, timeout, firewall - Solution: Retry with debug logging 4. Diagnostic Commands Reference: - doctor: Quick OAuth detection check - auth status: View token status - upstream list: Check connection status - upstream logs: View OAuth flow logs - auth login --log-level=debug: Test with debug output - upstream list --format=json | jq: Verify OAuth config This addresses user confusion about "Pending Auth" being displayed as an error state. Documentation now clearly explains it's a normal waiting state and provides step-by-step troubleshooting for all OAuth-related issues. Related to PR smart-mcp-proxy#165: Zero-config OAuth with RFC 8707 support Task: Update documentation for OAuth server states and troubleshooting
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
