Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 65 additions & 0 deletions cmd/mcpproxy/logdir.go
Original file line number Diff line number Diff line change
@@ -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
}
91 changes: 91 additions & 0 deletions cmd/mcpproxy/logdir_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
})
}
}
10 changes: 6 additions & 4 deletions cmd/mcpproxy/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
9 changes: 8 additions & 1 deletion docs/logging.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
22 changes: 14 additions & 8 deletions internal/logs/e2e_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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)
}
}

Expand Down
Loading