refactor: Extract shared LogConnectionError to eliminate duplicate connection error diagnostics (#2524)

Connection error diagnostic logging was duplicated across
`mcp/connection.go` and `launcher/log_helpers.go` — each independently
printing command/args, error, stderr, and hint analysis for known error
strings (`executable file not found`, `EOF`, `broken pipe`). Adding a
new diagnostic hint required updating both sites; divergence meant
inconsistent output depending on which code path triggered the failure.

## Changes

- **`internal/mcp/errors.go`** (new): Defines `ConnectionErrorContext`
and `LogConnectionError` — the single source of truth for connection
failure diagnostics. Merges hint logic from both original sites:
containerization hints (`IsDirectCommand` + `RunningInContainer`) and
error-string hints. Fields omit gracefully when zero-valued, so both
callers can populate only what they know.

- **`internal/mcp/connection.go`**: Replaces ~35-line inline error block
with a `LogConnectionError` call passing `{ServerID, Command, Args,
StderrOutput}`.

- **`internal/launcher/log_helpers.go`**: `logLaunchError` delegates
entirely to `mcp.LogConnectionError` with full launcher context
(`SessionID`, `Env`, `RunningInContainer`, `IsDirectCommand`,
`StartupTimeout`).

- **`internal/mcp/errors_test.go`** (new): Tests covering all hint
branches, stderr rendering, arg sanitization, and edge cases.

```go
// Before: each site had its own copy of this logic
if strings.Contains(err.Error(), "executable file not found") || ... {
    log.Printf("⚠️  Command '%s' not found in PATH", command)
    ...
}

// After: one shared function with full context
mcp.LogConnectionError(mcp.ConnectionErrorContext{
    ServerID:           serverID,
    SessionID:          sessionID,
    Command:            serverCfg.Command,
    Args:               serverCfg.Args,
    Env:                serverCfg.Env,
    RunningInContainer: l.runningInContainer,
    IsDirectCommand:    isDirectCommand,
    StartupTimeout:     l.startupTimeout,
}, err)
```

> [!WARNING]
>
> <details>
> <summary>Firewall rules blocked me from connecting to one or more
addresses (expand for details)</summary>
>
> #### I tried to connect to the following addresses, but was blocked by
firewall rules:
>
> - `example.com`
> - Triggering command: `/tmp/go-build3051059008/b329/launcher.test
/tmp/go-build3051059008/b329/launcher.test
-test.testlogfile=/tmp/go-build3051059008/b329/testlog.txt
-test.paniconexit0 -test.timeout=10m0s -test.v=true go dsa/ecdsa.go
p/bin/bash` (dns block)
> - Triggering command: `/tmp/go-build2944181223/b333/launcher.test
/tmp/go-build2944181223/b333/launcher.test
-test.testlogfile=/tmp/go-build2944181223/b333/testlog.txt
-test.paniconexit0 -test.timeout=10m0s ortc�� d -n 10 stmain.go
ache/go/1.25.8/x64/pkg/tool/linux_amd64/link (compatibility issue with
Go 1.25.0). Continuing with other checks...&#34;; \
elif command -v golan
telabs/wazero/in/tmp/go-build4284220926/b202/vet.cfg --64
ache/go/1.25.8/x64/pkg/tool/linux_amd64/link -I 1059008/b224/cmd.test
/launcher/log_helpers.go ker/docker-init -pthread
-Wl,--no-gc-sectdocker-cli-plugin-metadata -fmessage-length=0
ker/docker-init` (dns block)
> - Triggering command: `/tmp/go-build2960403912/b329/launcher.test
/tmp/go-build2960403912/b329/launcher.test
-test.testlogfile=/tmp/go-build2960403912/b329/testlog.txt
-test.paniconexit0 -test.timeout=10m0s -test.v=true
4181223/b314/_pk/run/containerd/io.containerd.runtime.v2.task/moby/83010862c130074fa53cbdec2f66a/usr/lib/networkd-dispatcher/off.d/chrony-onoffline
by/3d68fe704b33a--log-format by/a44f1c6d2c345json
by/3d68fe704b33a/usr/lib/networkd-dispatcher/off.d/chrony-onoffline
300bcc2a4ea80026start
64/pkg/tool/linu83010862c130074fa53cbdec2f66a97b6b568134d8551ff471fd0b14e7d12390
/opt/hostedtoolcache/go/1.25.8/x/var/run/docker/runtime-runc/moby d945��
irdJ/-TWaY8JNiNqMr3rjirdJ 64/pkg/tool/linux_amd64/asm
/opt/hostedtoolcache/go/1.25.8/xjson
/tmp/go-build305/usr/lib/open-iscsi/net-interface-handler 1059008/b165/
x_amd64/compile 4181223/b314/importcfg` (dns block)
> - `invalid-host-that-does-not-exist-12345.com`
> - Triggering command: `/tmp/go-build3051059008/b314/config.test
/tmp/go-build3051059008/b314/config.test
-test.testlogfile=/tmp/go-build3051059008/b314/testlog.txt
-test.paniconexit0 -test.timeout=10m0s -test.v=true go Pe6rubutY
x_amd64/vet
copilot.original/opt/hostedtoolcache/go/1.25.8/x64/pkg/tool/linux_amd64/vet`
(dns block)
> - Triggering command: `/tmp/go-build2944181223/b318/config.test
/tmp/go-build2944181223/b318/config.test
-test.testlogfile=/tmp/go-build2944181223/b318/testlog.txt
-test.paniconexit0 -test.timeout=10m0s` (dns block)
> - `nonexistent.local`
> - Triggering command: `/tmp/go-build3051059008/b329/launcher.test
/tmp/go-build3051059008/b329/launcher.test
-test.testlogfile=/tmp/go-build3051059008/b329/testlog.txt
-test.paniconexit0 -test.timeout=10m0s -test.v=true go dsa/ecdsa.go
p/bin/bash` (dns block)
> - Triggering command: `/tmp/go-build2944181223/b333/launcher.test
/tmp/go-build2944181223/b333/launcher.test
-test.testlogfile=/tmp/go-build2944181223/b333/testlog.txt
-test.paniconexit0 -test.timeout=10m0s ortc�� d -n 10 stmain.go
ache/go/1.25.8/x64/pkg/tool/linux_amd64/link (compatibility issue with
Go 1.25.0). Continuing with other checks...&#34;; \
elif command -v golan
telabs/wazero/in/tmp/go-build4284220926/b202/vet.cfg --64
ache/go/1.25.8/x64/pkg/tool/linux_amd64/link -I 1059008/b224/cmd.test
/launcher/log_helpers.go ker/docker-init -pthread
-Wl,--no-gc-sectdocker-cli-plugin-metadata -fmessage-length=0
ker/docker-init` (dns block)
> - Triggering command: `/tmp/go-build2960403912/b329/launcher.test
/tmp/go-build2960403912/b329/launcher.test
-test.testlogfile=/tmp/go-build2960403912/b329/testlog.txt
-test.paniconexit0 -test.timeout=10m0s -test.v=true
4181223/b314/_pk/run/containerd/io.containerd.runtime.v2.task/moby/83010862c130074fa53cbdec2f66a/usr/lib/networkd-dispatcher/off.d/chrony-onoffline
by/3d68fe704b33a--log-format by/a44f1c6d2c345json
by/3d68fe704b33a/usr/lib/networkd-dispatcher/off.d/chrony-onoffline
300bcc2a4ea80026start
64/pkg/tool/linu83010862c130074fa53cbdec2f66a97b6b568134d8551ff471fd0b14e7d12390
/opt/hostedtoolcache/go/1.25.8/x/var/run/docker/runtime-runc/moby d945��
irdJ/-TWaY8JNiNqMr3rjirdJ 64/pkg/tool/linux_amd64/asm
/opt/hostedtoolcache/go/1.25.8/xjson
/tmp/go-build305/usr/lib/open-iscsi/net-interface-handler 1059008/b165/
x_amd64/compile 4181223/b314/importcfg` (dns block)
> - `slow.example.com`
> - Triggering command: `/tmp/go-build3051059008/b329/launcher.test
/tmp/go-build3051059008/b329/launcher.test
-test.testlogfile=/tmp/go-build3051059008/b329/testlog.txt
-test.paniconexit0 -test.timeout=10m0s -test.v=true go dsa/ecdsa.go
p/bin/bash` (dns block)
> - Triggering command: `/tmp/go-build2944181223/b333/launcher.test
/tmp/go-build2944181223/b333/launcher.test
-test.testlogfile=/tmp/go-build2944181223/b333/testlog.txt
-test.paniconexit0 -test.timeout=10m0s ortc�� d -n 10 stmain.go
ache/go/1.25.8/x64/pkg/tool/linux_amd64/link (compatibility issue with
Go 1.25.0). Continuing with other checks...&#34;; \
elif command -v golan
telabs/wazero/in/tmp/go-build4284220926/b202/vet.cfg --64
ache/go/1.25.8/x64/pkg/tool/linux_amd64/link -I 1059008/b224/cmd.test
/launcher/log_helpers.go ker/docker-init -pthread
-Wl,--no-gc-sectdocker-cli-plugin-metadata -fmessage-length=0
ker/docker-init` (dns block)
> - Triggering command: `/tmp/go-build2960403912/b329/launcher.test
/tmp/go-build2960403912/b329/launcher.test
-test.testlogfile=/tmp/go-build2960403912/b329/testlog.txt
-test.paniconexit0 -test.timeout=10m0s -test.v=true
4181223/b314/_pk/run/containerd/io.containerd.runtime.v2.task/moby/83010862c130074fa53cbdec2f66a/usr/lib/networkd-dispatcher/off.d/chrony-onoffline
by/3d68fe704b33a--log-format by/a44f1c6d2c345json
by/3d68fe704b33a/usr/lib/networkd-dispatcher/off.d/chrony-onoffline
300bcc2a4ea80026start
64/pkg/tool/linu83010862c130074fa53cbdec2f66a97b6b568134d8551ff471fd0b14e7d12390
/opt/hostedtoolcache/go/1.25.8/x/var/run/docker/runtime-runc/moby d945��
irdJ/-TWaY8JNiNqMr3rjirdJ 64/pkg/tool/linux_amd64/asm
/opt/hostedtoolcache/go/1.25.8/xjson
/tmp/go-build305/usr/lib/open-iscsi/net-interface-handler 1059008/b165/
x_amd64/compile 4181223/b314/importcfg` (dns block)
> - `this-host-does-not-exist-12345.com`
> - Triggering command: `/tmp/go-build3051059008/b338/mcp.test
/tmp/go-build3051059008/b338/mcp.test
-test.testlogfile=/tmp/go-build3051059008/b338/testlog.txt
-test.paniconexit0 -test.timeout=10m0s -test.v=true 64/src/runtime/c-p
zGGbXQmyc ache/go/1.25.8/x-lang=go1.24` (dns block)
> - Triggering command: `/tmp/go-build2944181223/b342/mcp.test
/tmp/go-build2944181223/b342/mcp.test
-test.testlogfile=/tmp/go-build2944181223/b342/testlog.txt
-test.paniconexit0 -test.timeout=10m0s -uns��
pkg/mod/github.com/segmentio/asm@v1.1.3/cpu/arm6&#43;30
/tmp/go-build3051059008/b122/vet.cfg x_amd64/link -b
copilot/duplicat/usr/bin/runc 64/bin/bash x_amd64/link -o
/tmp/go-build3051059008/b297/_pkg_.a -trimpath x_amd64/vet -p
qV/wxj7iTUpwRLRu/usr/bin/runc -lang=go1.17 x_amd64/vet` (dns block)
> - Triggering command: `/tmp/go-build2960403912/b338/mcp.test
/tmp/go-build2960403912/b338/mcp.test
-test.testlogfile=/tmp/go-build2960403912/b338/testlog.txt
-test.paniconexit0 -test.timeout=10m0s -test.v=true` (dns block)
>
> If you need me to access, download, or install something from one of
these locations, you can either:
>
> - Configure [Actions setup
steps](https://gh.io/copilot/actions-setup-steps) to set up my
environment, which run before the firewall is enabled
> - Add the appropriate URLs or hosts to the custom allowlist in this
repository's [Copilot coding agent
settings](https://github.com/github/gh-aw-mcpg/settings/copilot/coding_agent)
(admins only)
>
> </details>

<!-- START COPILOT CODING AGENT TIPS -->
---

📍 Connect Copilot coding agent with [Jira](https://gh.io/cca-jira-docs),
[Azure Boards](https://gh.io/cca-azure-boards-docs) or
[Linear](https://gh.io/cca-linear-docs) to delegate work to Copilot in
one click without leaving your project management tool.

Author: lpcox

internal/launcher/log_helpers.go

@@ -9,6 +9,7 @@ import (
 	"github.com/github/gh-aw-mcpg/internal/config"
 	"github.com/github/gh-aw-mcpg/internal/logger"
 	"github.com/github/gh-aw-mcpg/internal/logger/sanitize"
+	"github.com/github/gh-aw-mcpg/internal/mcp"
 )
 
 // sessionSuffix returns a formatted session suffix for log messages
@@ -69,29 +70,16 @@ func (l *Launcher) logEnvPassthrough(args []string) {
 
 // logLaunchError logs detailed launch failure diagnostics
 func (l *Launcher) logLaunchError(serverID, sessionID string, err error, serverCfg *config.ServerConfig, isDirectCommand bool) {
-	logger.LogErrorWithServer(serverID, "backend", "Failed to launch MCP backend server%s: server=%s%s, error=%v",
-		sessionSuffix(sessionID), serverID, sessionSuffix(sessionID), err)
-	log.Printf("[LAUNCHER] ❌ FAILED to launch server '%s'%s", serverID, sessionSuffix(sessionID))
-	log.Printf("[LAUNCHER] Error: %v", err)
-	log.Printf("[LAUNCHER] Debug Information:")
-	log.Printf("[LAUNCHER]   - Command: %s", serverCfg.Command)
-	log.Printf("[LAUNCHER]   - Args: %v", sanitize.SanitizeArgs(serverCfg.Args))
-	log.Printf("[LAUNCHER]   - Env vars: %v", sanitize.TruncateSecretMap(serverCfg.Env))
-	log.Printf("[LAUNCHER]   - Running in container: %v", l.runningInContainer)
-	log.Printf("[LAUNCHER]   - Is direct command: %v", isDirectCommand)
-	log.Printf("[LAUNCHER]   - Startup timeout: %v", l.startupTimeout)
-
-	if isDirectCommand && l.runningInContainer {
-		log.Printf("[LAUNCHER] ⚠️  Possible causes:")
-		log.Printf("[LAUNCHER]   - Command '%s' may not be installed in the gateway container", serverCfg.Command)
-		log.Printf("[LAUNCHER]   - Consider using 'container' config instead of 'command'")
-		log.Printf("[LAUNCHER]   - Or add '%s' to the gateway's Dockerfile", serverCfg.Command)
-	} else if isDirectCommand {
-		log.Printf("[LAUNCHER] ⚠️  Possible causes:")
-		log.Printf("[LAUNCHER]   - Command '%s' may not be in PATH", serverCfg.Command)
-		log.Printf("[LAUNCHER]   - Check if '%s' is installed: which %s", serverCfg.Command, serverCfg.Command)
-		log.Printf("[LAUNCHER]   - Verify file permissions and execute bit")
-	}
+	mcp.LogConnectionError(mcp.ConnectionErrorContext{
+		ServerID:           serverID,
+		SessionID:          sessionID,
+		Command:            serverCfg.Command,
+		Args:               serverCfg.Args,
+		Env:                serverCfg.Env,
+		RunningInContainer: l.runningInContainer,
+		IsDirectCommand:    isDirectCommand,
+		StartupTimeout:     l.startupTimeout,
+	}, err)
 }
 
 // logTimeoutError logs startup timeout diagnostics

internal/mcp/connection.go

@@ -131,38 +131,13 @@ func NewConnection(ctx context.Context, serverID, command string, args []string,
 		cancel()
 		stderrPipeWriter.Close() // Close pipe to stop the stderr streaming goroutine
 
-		// Enhanced error context for debugging
-		logger.LogErrorMd("backend", "MCP backend connection failed, command=%s, args=%v, error=%v", command, sanitize.SanitizeArgs(expandedArgs), err)
-		log.Printf("❌ MCP Connection Failed:")
-		log.Printf("   Command: %s", command)
-		log.Printf("   Args: %v", sanitize.SanitizeArgs(expandedArgs))
-		log.Printf("   Error: %v", err)
-
-		// Log captured stderr output from the container/process
 		stderrOutput := strings.TrimSpace(stderrBuf.String())
-		if stderrOutput != "" {
-			sanitizedStderr := sanitize.SanitizeString(stderrOutput)
-			logger.LogErrorMd("backend", "MCP backend stderr output:\n%s", sanitizedStderr)
-			log.Printf("   📋 Container/Process stderr output:")
-			for _, line := range strings.Split(sanitizedStderr, "\n") {
-				log.Printf("      %s", line)
-			}
-		}
-
-		// Check if it's a command not found error
-		if strings.Contains(err.Error(), "executable file not found") ||
-			strings.Contains(err.Error(), "no such file or directory") {
-			logger.LogErrorMd("backend", "MCP backend command not found, command=%s", command)
-			log.Printf("   ⚠️  Command '%s' not found in PATH", command)
-			log.Printf("   ⚠️  Verify the command is installed and executable")
-		}
-
-		// Check if it's a connection/protocol error
-		if strings.Contains(err.Error(), "EOF") || strings.Contains(err.Error(), "broken pipe") {
-			logger.LogErrorMd("backend", "MCP backend connection/protocol error, command=%s", command)
-			log.Printf("   ⚠️  Process started but terminated unexpectedly")
-			log.Printf("   ⚠️  Check if the command supports MCP protocol over stdio")
-		}
+		LogConnectionError(ConnectionErrorContext{
+			ServerID:     serverID,
+			Command:      command,
+			Args:         expandedArgs,
+			StderrOutput: stderrOutput,
+		}, err)
 
 		logConn.Printf("Connection failed: command=%s, error=%v", command, err)
 		return nil, fmt.Errorf("failed to connect: %w", err)

internal/mcp/errors.go

@@ -0,0 +1,104 @@
+package mcp
+
+import (
+	"log"
+	"strings"
+	"time"
+
+	"github.com/github/gh-aw-mcpg/internal/logger"
+	"github.com/github/gh-aw-mcpg/internal/logger/sanitize"
+)
+
+// ConnectionErrorContext holds all context needed to produce a detailed connection
+// failure diagnostic. Fields left at their zero values are omitted from the output.
+type ConnectionErrorContext struct {
+	ServerID           string
+	SessionID          string
+	Command            string
+	Args               []string
+	Env                map[string]string
+	RunningInContainer bool
+	IsDirectCommand    bool
+	StartupTimeout     time.Duration
+	StderrOutput       string
+}
+
+// LogConnectionError logs detailed diagnostics for a connection failure, including
+// command context, captured stderr, and actionable hints based on the error type
+// and execution environment. All callers (launcher and mcp connection) use this
+// single function so that hint analysis and output format remain consistent.
+func LogConnectionError(errCtx ConnectionErrorContext, err error) {
+	suffix := ""
+	if errCtx.SessionID != "" {
+		suffix = " for session '" + errCtx.SessionID + "'"
+	}
+
+	// Structured log via file logger.
+	if errCtx.ServerID != "" {
+		logger.LogErrorWithServer(errCtx.ServerID, "backend",
+			"MCP backend connection failed%s: server=%s, command=%s, args=%v, error=%v",
+			suffix, errCtx.ServerID, errCtx.Command, sanitize.SanitizeArgs(errCtx.Args), err)
+	} else {
+		logger.LogErrorMd("backend",
+			"MCP backend connection failed, command=%s, args=%v, error=%v",
+			errCtx.Command, sanitize.SanitizeArgs(errCtx.Args), err)
+	}
+
+	// Human-readable console output.
+	if errCtx.ServerID != "" {
+		log.Printf("[LAUNCHER] ❌ FAILED to connect to server '%s'%s", errCtx.ServerID, suffix)
+	} else {
+		log.Printf("[LAUNCHER] ❌ MCP Connection Failed")
+	}
+	log.Printf("[LAUNCHER] Error: %v", err)
+	log.Printf("[LAUNCHER] Debug Information:")
+	log.Printf("[LAUNCHER]   - Command: %s", errCtx.Command)
+	log.Printf("[LAUNCHER]   - Args: %v", sanitize.SanitizeArgs(errCtx.Args))
+	if len(errCtx.Env) > 0 {
+		log.Printf("[LAUNCHER]   - Env vars: %v", sanitize.TruncateSecretMap(errCtx.Env))
+	}
+	if errCtx.RunningInContainer || errCtx.IsDirectCommand {
+		log.Printf("[LAUNCHER]   - Running in container: %v", errCtx.RunningInContainer)
+		log.Printf("[LAUNCHER]   - Is direct command: %v", errCtx.IsDirectCommand)
+	}
+	if errCtx.StartupTimeout > 0 {
+		log.Printf("[LAUNCHER]   - Startup timeout: %v", errCtx.StartupTimeout)
+	}
+
+	// Log captured stderr output from the container/process.
+	if errCtx.StderrOutput != "" {
+		sanitizedStderr := sanitize.SanitizeString(errCtx.StderrOutput)
+		logger.LogErrorMd("backend", "MCP backend stderr output:\n%s", sanitizedStderr)
+		log.Printf("[LAUNCHER]   📋 Process stderr output:")
+		for _, line := range strings.Split(sanitizedStderr, "\n") {
+			log.Printf("[LAUNCHER]      %s", line)
+		}
+	}
+
+	// Hints based on execution context (launcher-specific).
+	if errCtx.IsDirectCommand && errCtx.RunningInContainer {
+		log.Printf("[LAUNCHER] ⚠️  Possible causes:")
+		log.Printf("[LAUNCHER]   - Command '%s' may not be installed in the gateway container", errCtx.Command)
+		log.Printf("[LAUNCHER]   - Consider using 'container' config instead of 'command'")
+		log.Printf("[LAUNCHER]   - Or add '%s' to the gateway's Dockerfile", errCtx.Command)
+	} else if errCtx.IsDirectCommand {
+		log.Printf("[LAUNCHER] ⚠️  Possible causes:")
+		log.Printf("[LAUNCHER]   - Command '%s' may not be in PATH", errCtx.Command)
+		log.Printf("[LAUNCHER]   - Check if '%s' is installed: which %s", errCtx.Command, errCtx.Command)
+		log.Printf("[LAUNCHER]   - Verify file permissions and execute bit")
+	}
+
+	// Hints based on error message content.
+	errStr := err.Error()
+	if strings.Contains(errStr, "executable file not found") || strings.Contains(errStr, "no such file or directory") {
+		logger.LogErrorMd("backend", "MCP backend command not found, command=%s", errCtx.Command)
+		log.Printf("[LAUNCHER] ⚠️  Command '%s' not found in PATH", errCtx.Command)
+		log.Printf("[LAUNCHER] ⚠️  Verify the command is installed and executable")
+	}
+
+	if strings.Contains(errStr, "EOF") || strings.Contains(errStr, "broken pipe") {
+		logger.LogErrorMd("backend", "MCP backend connection/protocol error, command=%s", errCtx.Command)
+		log.Printf("[LAUNCHER] ⚠️  Process started but terminated unexpectedly")
+		log.Printf("[LAUNCHER] ⚠️  Check if the command supports MCP protocol over stdio")
+	}
+}