fix(logs): isolate non-default data-dir logs to <data-dir>/logs (#657)

The phantom "core restarts every ~10s" reports (MCP-2250, split from
MCP-2207) were a logging artifact, not a real restart loop. `serve`
enables file logging by default and derives the log dir purely from
$HOME (internal/logs.GetLogDir), ignoring --data-dir. So every
`mcpproxy serve` run — Go integration tests (temp data-dirs), e2e
scripts, FE/QA Playwright harnesses, AND the real prod core — appended
to the same ~/Library/Logs/mcpproxy/main.log. A reader of that shared
file saw dozens of fast, interleaved boots and concluded the core was
restarting every ~10s. Forensics on a 72-min window show only 2 real
prod-core boots (data_dir ~/.mcpproxy) among ~42, and every shutdown
was reason="signal" (no panics/self-restarts).

Fix: when the resolved data dir is non-default and no explicit
--log-dir was given, co-locate logs under <data-dir>/logs. The default
data dir (~/.mcpproxy) is unchanged and still logs to the OS-standard
location, so the tray and documented paths keep working. An explicit
--log-dir still wins.

- Add resolveServeLogDir helper (cmd/mcpproxy/logdir.go) with unit tests
  covering the full precedence: --log-dir > config log_dir >
  <data-dir>/logs (non-default) > OS-standard.
- Wire it into runServer (cmd/mcpproxy/main.go).
- Update TestE2E_MCPProxyWithLogging to assert the binary writes to
  <data-dir>/logs and does NOT leak into the shared OS-standard dir.
- Document the resolution order in docs/logging.md.

Related MCP-2250

Co-authored-by: Paperclip <noreply@paperclip.ing>

Author: Dumbris

cmd/mcpproxy/logdir.go

@@ -0,0 +1,65 @@
+package main
+
+import (
+	"os"
+	"path/filepath"
+
+	"github.com/smart-mcp-proxy/mcpproxy-go/internal/config"
+)
+
+// defaultDataDirPath returns the default data directory (<home>/.mcpproxy),
+// mirroring internal/config's default resolution. It is used to decide whether
+// the resolved data dir is "non-default" for log co-location. On failure to
+// resolve the home dir it returns config.DefaultDataDir, which will simply not
+// match any absolute data dir and leave logs at the OS-standard location.
+func defaultDataDirPath() string {
+	home, err := os.UserHomeDir()
+	if err != nil {
+		return config.DefaultDataDir
+	}
+	return filepath.Join(home, config.DefaultDataDir)
+}
+
+// resolveServeLogDir decides the log directory for the `serve` command.
+//
+// Precedence:
+//  1. explicit --log-dir flag (explicitLogDir) — always wins.
+//  2. a log dir already set in the loaded config (configLogDir).
+//  3. for a NON-default data dir, co-locate logs under <data-dir>/logs.
+//  4. otherwise "" — meaning the OS-standard location resolved by
+//     internal/logs.GetLogDir (e.g. ~/Library/Logs/mcpproxy on macOS).
+//
+// Step 3 is the fix for MCP-2250: Go integration tests, e2e scripts, and the
+// FE/QA Playwright harnesses all run `mcpproxy serve` with a custom data dir
+// (temp dirs, ./test-data, /tmp/mcpproxy-*) but, because the log dir was
+// derived purely from $HOME, every one of them appended to the SAME shared
+// prod log at ~/Library/Logs/mcpproxy/main.log. A reader of that file saw
+// dozens of fast boots interleaved and mistook it for the core restarting
+// ~every 10s. Co-locating logs with a non-default data dir keeps those runs
+// self-contained and leaves the real prod log clean. The default data dir
+// (~/.mcpproxy) is unchanged: it still logs to the OS-standard location so the
+// tray and documented paths keep working.
+func resolveServeLogDir(explicitLogDir, configLogDir, dataDir, defaultDataDir string) string {
+	if explicitLogDir != "" {
+		return explicitLogDir
+	}
+	if configLogDir != "" {
+		return configLogDir
+	}
+	if dataDir != "" && !sameDir(dataDir, defaultDataDir) {
+		return filepath.Join(dataDir, "logs")
+	}
+	return ""
+}
+
+// sameDir reports whether two paths refer to the same directory, comparing
+// their cleaned absolute forms so that relative/aliased spellings of the
+// default data dir are still recognized as the default.
+func sameDir(a, b string) bool {
+	absA, errA := filepath.Abs(a)
+	absB, errB := filepath.Abs(b)
+	if errA != nil || errB != nil {
+		return filepath.Clean(a) == filepath.Clean(b)
+	}
+	return absA == absB
+}

cmd/mcpproxy/logdir_test.go

@@ -0,0 +1,91 @@
+package main
+
+import (
+	"path/filepath"
+	"testing"
+)
+
+// TestResolveServeLogDir documents the precedence that keeps non-default-data-dir
+// runs (Go tests, e2e scripts, FE/QA harnesses) from polluting the shared prod
+// log at ~/Library/Logs/mcpproxy/main.log — the root cause of the phantom
+// "core restarts every 10s" reports in MCP-2250.
+func TestResolveServeLogDir(t *testing.T) {
+	const defaultDataDir = "/home/u/.mcpproxy"
+
+	cases := []struct {
+		name        string
+		explicit    string
+		configLog   string
+		dataDir     string
+		defaultData string
+		want        string
+	}{
+		{
+			name:        "explicit --log-dir always wins",
+			explicit:    "/custom/logs",
+			configLog:   "/cfg/logs",
+			dataDir:     "/tmp/test-123",
+			defaultData: defaultDataDir,
+			want:        "/custom/logs",
+		},
+		{
+			name:        "config Logging.LogDir wins when no flag",
+			explicit:    "",
+			configLog:   "/cfg/logs",
+			dataDir:     "/tmp/test-123",
+			defaultData: defaultDataDir,
+			want:        "/cfg/logs",
+		},
+		{
+			name:        "default data dir keeps OS-standard log dir (empty => GetLogDir)",
+			explicit:    "",
+			configLog:   "",
+			dataDir:     defaultDataDir,
+			defaultData: defaultDataDir,
+			want:        "",
+		},
+		{
+			name:        "non-default data dir co-locates logs under <data-dir>/logs",
+			explicit:    "",
+			configLog:   "",
+			dataDir:     "/tmp/mcpproxy-test-Foo",
+			defaultData: defaultDataDir,
+			want:        filepath.Join("/tmp/mcpproxy-test-Foo", "logs"),
+		},
+		{
+			name:        "relative non-default data dir (harness ./test-data) co-locates",
+			explicit:    "",
+			configLog:   "",
+			dataDir:     "./test-data",
+			defaultData: defaultDataDir,
+			want:        filepath.Join("test-data", "logs"),
+		},
+		{
+			name:        "absolute spelling of default data dir is treated as default",
+			explicit:    "",
+			configLog:   "",
+			dataDir:     "/home/u/../u/.mcpproxy",
+			defaultData: defaultDataDir,
+			want:        "",
+		},
+		{
+			name:        "empty data dir is a no-op (OS-standard)",
+			explicit:    "",
+			configLog:   "",
+			dataDir:     "",
+			defaultData: defaultDataDir,
+			want:        "",
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			got := resolveServeLogDir(tc.explicit, tc.configLog, tc.dataDir, tc.defaultData)
+			// Compare cleaned, since the helper may return a joined relative path.
+			if filepath.Clean(got) != filepath.Clean(tc.want) {
+				t.Fatalf("resolveServeLogDir(%q,%q,%q,%q) = %q, want %q",
+					tc.explicit, tc.configLog, tc.dataDir, tc.defaultData, got, tc.want)
+			}
+		})
+	}
+}

cmd/mcpproxy/main.go

@@ -466,10 +466,12 @@ func runServer(cmd *cobra.Command, _ []string) error {
 		}
 	}
 
-	// Override log directory if specified
-	if cmdLogDir != "" {
-		cfg.Logging.LogDir = cmdLogDir
-	}
+	// Resolve the log directory. An explicit --log-dir wins; otherwise a
+	// non-default data dir co-locates logs under <data-dir>/logs so that
+	// tests/e2e/harness `serve` runs do not pollute the shared OS-standard
+	// prod log (root cause of the phantom "core restarts every 10s" in
+	// MCP-2250). The default data dir keeps the OS-standard location.
+	cfg.Logging.LogDir = resolveServeLogDir(cmdLogDir, cfg.Logging.LogDir, cfg.DataDir, defaultDataDirPath())
 
 	// Setup logger with new logging system
 	logger, err := logs.SetupLogger(cfg.Logging)

docs/logging.md

@@ -73,6 +73,13 @@ The logging system automatically selects the appropriate directory based on your
 - **Location**: `~/.mcpproxy/logs/`
 - **Used when**: OS detection fails or standard directories are inaccessible
 
+### Custom data directory
+- **Location**: `<data-dir>/logs/`
+- **Used when**: the resolved data directory is **not** the default (`~/.mcpproxy`) — i.e. you ran with a custom `--data-dir` flag or a `data_dir` set in the config file — **and** no explicit `--log-dir` was given.
+- **Rationale**: co-locating logs with a custom data directory keeps non-default runs (integration tests, e2e scripts, throwaway/QA instances) self-contained so they never write into the shared OS-standard log (`~/Library/Logs/mcpproxy/main.log`). Mixing many short-lived `serve` processes into that single shared file previously made the log unreadable and produced misleading "the core restarts every ~10s" signals.
+- **Override**: an explicit `--log-dir` always takes precedence; the default data directory (`~/.mcpproxy`) is unaffected and still logs to the OS-standard location above.
+- **Resolution order**: `--log-dir` flag → `logging.log_dir` in config → `<data-dir>/logs` (non-default data dir) → OS-standard location.
+
 ## Configuration Options
 
 ### Log Levels
@@ -383,4 +390,4 @@ The logging system follows established standards for each operating system:
 - **Windows**: [Application Data Guidelines](https://docs.microsoft.com/en-us/windows/win32/shell/knownfolderid)
 - **Linux**: [XDG Base Directory Specification](https://specifications.freedesktop.org/basedir-spec/basedir-spec-latest.html)
 
-This ensures that logs are stored in the expected locations for each platform, making them easy to find and manage. 
+This ensures that logs are stored in the expected locations for each platform, making them easy to find and manage.

internal/logs/e2e_test.go

@@ -336,12 +336,21 @@ func TestE2E_MCPProxyWithLogging(t *testing.T) {
 	// Wait for the command to finish
 	_ = cmd.Wait()
 
-	// Check if log file was created
-	logFilePath, err := GetLogFilePath("mcpproxy-e2e-binary.log")
-	require.NoError(t, err)
-
+	// Because the config sets a non-default data_dir (tempDir) and no
+	// --log-dir was given, logs are co-located under <data-dir>/logs rather
+	// than the shared OS-standard location (MCP-2250). This keeps test/e2e
+	// runs from polluting the real ~/Library/Logs/mcpproxy/main.log.
+	logFilePath := filepath.Join(tempDir, "logs", "mcpproxy-e2e-binary.log")
 	_, err = os.Stat(logFilePath)
-	assert.NoError(t, err, "Log file should be created by mcpproxy binary")
+	assert.NoError(t, err, "Log file should be created under <data-dir>/logs by mcpproxy binary")
+
+	// The shared OS-standard location must NOT receive this run's log.
+	osStdPath, osErr := GetLogFilePath("mcpproxy-e2e-binary.log")
+	require.NoError(t, osErr)
+	if _, statErr := os.Stat(osStdPath); statErr == nil {
+		t.Errorf("custom data_dir run leaked its log into the shared OS-standard dir: %s", osStdPath)
+		os.Remove(osStdPath)
+	}
 
 	// Read and verify log content
 	if err == nil {
@@ -351,9 +360,6 @@ func TestE2E_MCPProxyWithLogging(t *testing.T) {
 		contentStr := string(content)
 		assert.Contains(t, contentStr, "Starting mcpproxy", "Log should contain startup message")
 		assert.Contains(t, contentStr, "Log directory configured", "Log should contain directory info")
-
-		// Clean up
-		os.Remove(logFilePath)
 	}
 }