Skip to content

Commit 5d49835

Browse files
fix(tests): E2E tests use isolated config and instance-scoped container cleanup
Two issues were causing E2E tests to fail: 1. Tests loaded user's real ~/.mcpproxy/mcp_config.json instead of a clean test config, causing connection attempts to 15+ real upstream servers. Fixed by creating minimal config files in test temp directories. 2. Docker container cleanup affected ALL mcpproxy instances on the machine, not just the test instance. This caused 15+ second shutdown delays as the test server tried to clean up containers from other running instances. Fixed by filtering container operations by instance ID label. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 41fccb9 commit 5d49835

3 files changed

Lines changed: 45 additions & 5 deletions

File tree

internal/server/info_shutdown_e2e_test.go

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,15 @@ func TestInfoEndpoint(t *testing.T) {
3636
err := os.Chmod(tempDir, 0700)
3737
require.NoError(t, err, "Failed to set secure permissions on temp directory")
3838

39+
// Create a minimal config file to avoid loading user's real config
40+
configPath := filepath.Join(tempDir, "mcp_config.json")
41+
minimalConfig := `{
42+
"listen": "127.0.0.1:0",
43+
"mcpServers": [],
44+
"docker_isolation": {"enabled": false}
45+
}`
46+
require.NoError(t, os.WriteFile(configPath, []byte(minimalConfig), 0600))
47+
3948
// Find available port
4049
ln, err := net.Listen("tcp", ":0")
4150
require.NoError(t, err)
@@ -49,6 +58,7 @@ func TestInfoEndpoint(t *testing.T) {
4958
defer cancel()
5059

5160
cmd := exec.CommandContext(ctx, binaryPath, "serve",
61+
"--config", configPath,
5262
"--data-dir", tempDir,
5363
"--listen", listenAddr)
5464

@@ -150,6 +160,15 @@ func TestGracefulShutdownNoPanic(t *testing.T) {
150160
err := os.Chmod(tempDir, 0700)
151161
require.NoError(t, err, "Failed to set secure permissions on temp directory")
152162

163+
// Create a minimal config file to avoid loading user's real config
164+
configPath := filepath.Join(tempDir, "mcp_config.json")
165+
minimalConfig := `{
166+
"listen": "127.0.0.1:0",
167+
"mcpServers": [],
168+
"docker_isolation": {"enabled": false}
169+
}`
170+
require.NoError(t, os.WriteFile(configPath, []byte(minimalConfig), 0600))
171+
153172
// Find available port
154173
ln, err := net.Listen("tcp", ":0")
155174
require.NoError(t, err)
@@ -163,6 +182,7 @@ func TestGracefulShutdownNoPanic(t *testing.T) {
163182
defer cancel()
164183

165184
cmd := exec.CommandContext(ctx, binaryPath, "serve",
185+
"--config", configPath,
166186
"--data-dir", tempDir,
167187
"--listen", listenAddr,
168188
"--log-level", "debug")
@@ -244,13 +264,23 @@ func TestSocketInfoEndpoint(t *testing.T) {
244264
err := os.Chmod(tempDir, 0700)
245265
require.NoError(t, err, "Failed to set secure permissions on temp directory")
246266

267+
// Create a minimal config file to avoid loading user's real config
268+
configPath := filepath.Join(tempDir, "mcp_config.json")
269+
minimalConfig := `{
270+
"listen": "127.0.0.1:0",
271+
"mcpServers": [],
272+
"docker_isolation": {"enabled": false}
273+
}`
274+
require.NoError(t, os.WriteFile(configPath, []byte(minimalConfig), 0600))
275+
247276
socketPath := filepath.Join(tempDir, "mcpproxy.sock")
248277

249278
ctx, cancel := context.WithCancel(context.Background())
250279
defer cancel()
251280

252281
// Start server with socket enabled
253282
cmd := exec.CommandContext(ctx, binaryPath, "serve",
283+
"--config", configPath,
254284
"--data-dir", tempDir,
255285
"--listen", "127.0.0.1:0", // Random port for HTTP
256286
"--enable-socket", "true")

internal/upstream/core/instance.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,11 @@ func getInstanceID() string {
3232
return instanceID
3333
}
3434

35+
// GetInstanceID returns the unique identifier for this mcpproxy instance (exported for use by manager)
36+
func GetInstanceID() string {
37+
return getInstanceID()
38+
}
39+
3540
// loadInstanceID attempts to load the instance ID from disk
3641
func loadInstanceID() (string, error) {
3742
instanceFile := filepath.Join(os.TempDir(), "mcpproxy-instance-id")

internal/upstream/manager.go

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -616,16 +616,19 @@ func (m *Manager) cleanupAllManagedContainers(ctx context.Context) {
616616

617617
// ForceCleanupAllContainers is a public wrapper for emergency container cleanup
618618
// This is called when graceful shutdown fails and containers must be force-removed
619+
// Only removes containers owned by THIS instance (matching instance ID)
619620
func (m *Manager) ForceCleanupAllContainers() {
620-
m.logger.Warn("Force cleanup requested - removing all managed containers immediately")
621+
m.logger.Warn("Force cleanup requested - removing all managed containers for this instance")
621622

622623
// Create a short-lived context for force cleanup (30 seconds max)
623624
ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
624625
defer cancel()
625626

626-
// Find all containers with our management label
627+
// Find all containers with our management label AND our instance ID
628+
instanceID := core.GetInstanceID()
627629
listCmd := exec.CommandContext(ctx, "docker", "ps", "-a",
628630
"--filter", "label=com.mcpproxy.managed=true",
631+
"--filter", fmt.Sprintf("label=com.mcpproxy.instance=%s", instanceID),
629632
"--format", "{{.ID}}\t{{.Names}}")
630633

631634
output, err := listCmd.Output()
@@ -1141,14 +1144,16 @@ func (m *Manager) DisconnectAll() error {
11411144
return nil
11421145
}
11431146

1144-
// HasDockerContainers checks if any Docker containers are actually running
1147+
// HasDockerContainers checks if any Docker containers owned by THIS instance are actually running
11451148
func (m *Manager) HasDockerContainers() bool {
1146-
// Check if any containers with our labels are actually running
1149+
// Check if any containers with our labels AND our instance ID are running
11471150
ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second)
11481151
defer cancel()
11491152

1153+
instanceID := core.GetInstanceID()
11501154
listCmd := exec.CommandContext(ctx, "docker", "ps", "-q",
1151-
"--filter", "label=com.mcpproxy.managed=true")
1155+
"--filter", "label=com.mcpproxy.managed=true",
1156+
"--filter", fmt.Sprintf("label=com.mcpproxy.instance=%s", instanceID))
11521157

11531158
output, err := listCmd.Output()
11541159
if err != nil {

0 commit comments

Comments
 (0)