diff --git a/cmd/mcpproxy/activity_cmd.go b/cmd/mcpproxy/activity_cmd.go index 4ab01379..18488063 100644 --- a/cmd/mcpproxy/activity_cmd.go +++ b/cmd/mcpproxy/activity_cmd.go @@ -780,14 +780,7 @@ func init() { // getActivityClient creates an HTTP client for the daemon func getActivityClient(logger *zap.SugaredLogger) (*cliclient.Client, error) { - // Load config - use explicit config file if provided via -c flag - var cfg *config.Config - var err error - if configFile != "" { - cfg, err = config.LoadFromFile(configFile) - } else { - cfg, err = config.Load() - } + cfg, err := loadActivityConfig() if err != nil { return nil, fmt.Errorf("failed to load config: %w", err) } @@ -966,12 +959,7 @@ func runActivityWatch(cmd *cobra.Command, _ []string) error { defer func() { _ = logger.Sync() }() // Load config to get endpoint - use same logic as getActivityClient - var cfg *config.Config - if configFile != "" { - cfg, err = config.LoadFromFile(configFile) - } else { - cfg, err = config.Load() - } + cfg, err := loadActivityConfig() if err != nil { return outputActivityError(err, "CONFIG_ERROR") } @@ -1566,12 +1554,7 @@ func runActivityExport(cmd *cobra.Command, _ []string) error { } // Load config - use explicit config file if provided via -c flag - var cfg *config.Config - if configFile != "" { - cfg, err = config.LoadFromFile(configFile) - } else { - cfg, err = config.Load() - } + cfg, err := loadActivityConfig() if err != nil { return outputActivityError(err, "CONFIG_ERROR") } diff --git a/cmd/mcpproxy/cli_config.go b/cmd/mcpproxy/cli_config.go new file mode 100644 index 00000000..b6feceef --- /dev/null +++ b/cmd/mcpproxy/cli_config.go @@ -0,0 +1,52 @@ +package main + +import ( + "github.com/smart-mcp-proxy/mcpproxy-go/internal/config" +) + +// loadCLIConfig loads a CLI command's config from explicitPath (the command's +// --config flag) when set, falling back to the default search path, and applies +// the global --data-dir flag on top (GH #854/#897/#908). Without the DataDir +// override, socket.DetectSocketPath probes the default data dir and the command +// either reports "daemon is not reachable" or silently talks to the wrong +// daemon instance. Every per-command loader below must go through this helper +// (enforced by TestLoadersHonorGlobalDataDirFlag). +func loadCLIConfig(explicitPath string) (*config.Config, error) { + var cfg *config.Config + var err error + if explicitPath != "" { + cfg, err = config.LoadFromFile(explicitPath) + } else { + cfg, err = config.Load() + } + if err != nil { + return nil, err + } + if dataDir != "" { + cfg.DataDir = dataDir + } + return cfg, nil +} + +// Per-command loaders for commands whose config flow previously used bare +// config.Load() and ignored --data-dir (GH #908). + +func loadActivityConfig() (*config.Config, error) { + return loadCLIConfig(configFile) +} + +func loadCredentialConfig() (*config.Config, error) { + return loadCLIConfig(configFile) +} + +func loadSecurityConfig() (*config.Config, error) { + return loadCLIConfig(configFile) +} + +func loadTrustCertConfig() (*config.Config, error) { + return loadCLIConfig(configFile) +} + +func loadTUIConfig() (*config.Config, error) { + return loadCLIConfig(configFile) +} diff --git a/cmd/mcpproxy/connect_cmd.go b/cmd/mcpproxy/connect_cmd.go index 4fd0b330..c083f322 100644 --- a/cmd/mcpproxy/connect_cmd.go +++ b/cmd/mcpproxy/connect_cmd.go @@ -272,22 +272,5 @@ func printConnectResult(result *connect.ConnectResult, formatter clioutput.Outpu } func loadConnectConfig() (*config.Config, error) { - if configFile != "" { - cfg, err := config.LoadFromFile(configFile) - if err != nil { - return nil, err - } - if dataDir != "" { - cfg.DataDir = dataDir - } - return cfg, nil - } - cfg, err := config.Load() - if err != nil { - return nil, err - } - if dataDir != "" { - cfg.DataDir = dataDir - } - return cfg, nil + return loadCLIConfig(configFile) } diff --git a/cmd/mcpproxy/credential_cmd.go b/cmd/mcpproxy/credential_cmd.go index 91b2bcf2..38302569 100644 --- a/cmd/mcpproxy/credential_cmd.go +++ b/cmd/mcpproxy/credential_cmd.go @@ -14,7 +14,6 @@ import ( "go.uber.org/zap" "github.com/smart-mcp-proxy/mcpproxy-go/internal/cliclient" - "github.com/smart-mcp-proxy/mcpproxy-go/internal/config" ) // Server-edition flags for the credential command group. The credential broker @@ -119,22 +118,29 @@ Examples: // --- client wiring --- // resolveCredentialBaseURL determines the server base URL from the --url flag, -// the MCPPROXY_SERVER_URL env var, or the local listen address in config. -func resolveCredentialBaseURL() string { +// the MCPPROXY_SERVER_URL env var, or the local listen address in config. An +// explicitly passed --config must fail loudly; only the implicit default-path +// load falls back to the default URL. +func resolveCredentialBaseURL() (string, error) { if credServerURL != "" { - return strings.TrimRight(credServerURL, "/") + return strings.TrimRight(credServerURL, "/"), nil } if env := os.Getenv("MCPPROXY_SERVER_URL"); env != "" { - return strings.TrimRight(env, "/") + return strings.TrimRight(env, "/"), nil } - if cfg, err := config.Load(); err == nil && cfg.Listen != "" { + cfg, err := loadCredentialConfig() + if err != nil { + if configFile != "" { + return "", fmt.Errorf("failed to load config: %w", err) + } + } else if cfg.Listen != "" { listen := cfg.Listen if strings.HasPrefix(listen, ":") { listen = "127.0.0.1" + listen } - return "http://" + listen + return "http://" + listen, nil } - return "http://127.0.0.1:8080" + return "http://127.0.0.1:8080", nil } // resolveCredentialToken returns the user JWT from --token or MCPPROXY_TOKEN. @@ -145,16 +151,22 @@ func resolveCredentialToken() string { return os.Getenv("MCPPROXY_TOKEN") } -func newCredentialClient() (*cliclient.Client, string) { - baseURL := resolveCredentialBaseURL() +func newCredentialClient() (*cliclient.Client, string, error) { + baseURL, err := resolveCredentialBaseURL() + if err != nil { + return nil, "", err + } logger, _ := zap.NewProduction() - return cliclient.NewClientWithBearer(baseURL, resolveCredentialToken(), logger.Sugar()), baseURL + return cliclient.NewClientWithBearer(baseURL, resolveCredentialToken(), logger.Sugar()), baseURL, nil } // --- run functions --- func runCredentialList(_ *cobra.Command, _ []string) error { - client, _ := newCredentialClient() + client, _, err := newCredentialClient() + if err != nil { + return err + } ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) defer cancel() @@ -166,7 +178,10 @@ func runCredentialList(_ *cobra.Command, _ []string) error { } func runCredentialStatus(_ *cobra.Command, args []string) error { - client, _ := newCredentialClient() + client, _, err := newCredentialClient() + if err != nil { + return err + } ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) defer cancel() @@ -182,7 +197,10 @@ func runCredentialStatus(_ *cobra.Command, args []string) error { } func runCredentialConnect(_ *cobra.Command, args []string) error { - baseURL := resolveCredentialBaseURL() + baseURL, err := resolveCredentialBaseURL() + if err != nil { + return err + } connectURL := credentialConnectURL(baseURL, args[0]) format := ResolveOutputFormat() @@ -196,7 +214,10 @@ func runCredentialConnect(_ *cobra.Command, args []string) error { } func runCredentialRemove(_ *cobra.Command, args []string) error { - client, _ := newCredentialClient() + client, _, err := newCredentialClient() + if err != nil { + return err + } ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) defer cancel() diff --git a/cmd/mcpproxy/credential_cmd_test.go b/cmd/mcpproxy/credential_cmd_test.go index 6cf6cee8..bfeda760 100644 --- a/cmd/mcpproxy/credential_cmd_test.go +++ b/cmd/mcpproxy/credential_cmd_test.go @@ -3,6 +3,7 @@ package main import ( + "path/filepath" "strings" "testing" "time" @@ -93,13 +94,24 @@ func TestCredentialConnectURL_EscapesServer(t *testing.T) { func TestResolveCredentialBaseURL_FlagAndEnv(t *testing.T) { t.Setenv("MCPPROXY_SERVER_URL", "https://env.example.com/") credServerURL = "" - if got := resolveCredentialBaseURL(); got != "https://env.example.com" { - t.Errorf("env base URL = %q", got) + if got, err := resolveCredentialBaseURL(); err != nil || got != "https://env.example.com" { + t.Errorf("env base URL = %q, err = %v", got, err) } credServerURL = "https://flag.example.com/" defer func() { credServerURL = "" }() - if got := resolveCredentialBaseURL(); got != "https://flag.example.com" { - t.Errorf("flag base URL = %q (flag should win)", got) + if got, err := resolveCredentialBaseURL(); err != nil || got != "https://flag.example.com" { + t.Errorf("flag base URL = %q, err = %v (flag should win)", got, err) + } +} + +func TestResolveCredentialBaseURL_ExplicitConfigErrors(t *testing.T) { + t.Setenv("MCPPROXY_SERVER_URL", "") + credServerURL = "" + oldConfigFile := configFile + configFile = filepath.Join(t.TempDir(), "missing.json") + defer func() { configFile = oldConfigFile }() + if got, err := resolveCredentialBaseURL(); err == nil { + t.Errorf("expected error for missing explicit --config, got base URL %q", got) } } diff --git a/cmd/mcpproxy/datadir_flag_test.go b/cmd/mcpproxy/datadir_flag_test.go index 9295d233..5e50e156 100644 --- a/cmd/mcpproxy/datadir_flag_test.go +++ b/cmd/mcpproxy/datadir_flag_test.go @@ -30,6 +30,17 @@ func TestLoadersHonorGlobalDataDirFlag(t *testing.T) { {"code", func(p string) { codeConfigPath = p }, loadCodeConfig}, {"tools", func(p string) { configPath = p }, loadToolsConfig}, {"token", func(p string) { tokenConfigPath = p }, loadTokenConfig}, + {"activity", func(p string) { configFile = p }, loadActivityConfig}, + {"connect", func(p string) { configFile = p }, loadConnectConfig}, + {"credential", func(p string) { configFile = p }, loadCredentialConfig}, + {"feedback", func(p string) { configFile = p }, loadFeedbackConfig}, + {"registry", func(p string) { registryConfigPath = p }, loadRegistryConfig}, + {"security", func(p string) { configFile = p }, loadSecurityConfig}, + {"status", func(p string) { configFile = p }, loadStatusConfig}, + {"telemetry", func(p string) { configFile = p }, loadTelemetryConfig}, + {"trust-cert", func(p string) { configFile = p }, loadTrustCertConfig}, + {"tui", func(p string) { configFile = p }, loadTUIConfig}, + {"upstream", func(p string) { upstreamConfigPath = p }, loadUpstreamConfig}, } for _, tc := range cases { diff --git a/cmd/mcpproxy/doctor_cmd.go b/cmd/mcpproxy/doctor_cmd.go index 062b08e0..12b751d3 100644 --- a/cmd/mcpproxy/doctor_cmd.go +++ b/cmd/mcpproxy/doctor_cmd.go @@ -527,26 +527,7 @@ func outputDiagnostics(diag map[string]interface{}, info map[string]interface{}, } func loadDoctorConfig() (*config.Config, error) { - if doctorConfigPath != "" { - cfg, err := config.LoadFromFile(doctorConfigPath) - if err != nil { - return nil, err - } - // Respect global --data-dir flag - if dataDir != "" { - cfg.DataDir = dataDir - } - return cfg, nil - } - cfg, err := config.Load() - if err != nil { - return nil, err - } - // Respect global --data-dir flag - if dataDir != "" { - cfg.DataDir = dataDir - } - return cfg, nil + return loadCLIConfig(doctorConfigPath) } func createDoctorLogger(level string) (*zap.Logger, error) { diff --git a/cmd/mcpproxy/feedback_cmd.go b/cmd/mcpproxy/feedback_cmd.go index 3b4f3d91..d5caca7b 100644 --- a/cmd/mcpproxy/feedback_cmd.go +++ b/cmd/mcpproxy/feedback_cmd.go @@ -100,22 +100,5 @@ func runFeedback(cmd *cobra.Command, args []string) error { } func loadFeedbackConfig() (*config.Config, error) { - if configFile != "" { - cfg, err := config.LoadFromFile(configFile) - if err != nil { - return nil, err - } - if dataDir != "" { - cfg.DataDir = dataDir - } - return cfg, nil - } - cfg, err := config.Load() - if err != nil { - return nil, err - } - if dataDir != "" { - cfg.DataDir = dataDir - } - return cfg, nil + return loadCLIConfig(configFile) } diff --git a/cmd/mcpproxy/security_cmd.go b/cmd/mcpproxy/security_cmd.go index 5e1fd3e8..bf331c61 100644 --- a/cmd/mcpproxy/security_cmd.go +++ b/cmd/mcpproxy/security_cmd.go @@ -79,21 +79,10 @@ Examples: // `mcpproxy security ...` commands behave consistently with `mcpproxy serve`, // `mcpproxy status`, and `mcpproxy upstream ...`. func newSecurityCLIClient() (*cliclient.Client, *config.Config, error) { - var ( - cfg *config.Config - err error - ) - if configFile != "" { - cfg, err = config.LoadFromFile(configFile) - } else { - cfg, err = config.Load() - } + cfg, err := loadSecurityConfig() if err != nil { return nil, nil, fmt.Errorf("failed to load config: %w", err) } - if dataDir != "" { - cfg.DataDir = dataDir - } logger, _ := zap.NewProduction() defer func() { _ = logger.Sync() }() diff --git a/cmd/mcpproxy/status_cmd.go b/cmd/mcpproxy/status_cmd.go index 982b3c56..07ea457d 100644 --- a/cmd/mcpproxy/status_cmd.go +++ b/cmd/mcpproxy/status_cmd.go @@ -530,22 +530,5 @@ func printStatusTable(info *StatusInfo) { } func loadStatusConfig() (*config.Config, error) { - if configFile != "" { - cfg, err := config.LoadFromFile(configFile) - if err != nil { - return nil, err - } - if dataDir != "" { - cfg.DataDir = dataDir - } - return cfg, nil - } - cfg, err := config.Load() - if err != nil { - return nil, err - } - if dataDir != "" { - cfg.DataDir = dataDir - } - return cfg, nil + return loadCLIConfig(configFile) } diff --git a/cmd/mcpproxy/telemetry_cmd.go b/cmd/mcpproxy/telemetry_cmd.go index 2966b5a5..c7915b87 100644 --- a/cmd/mcpproxy/telemetry_cmd.go +++ b/cmd/mcpproxy/telemetry_cmd.go @@ -311,24 +311,7 @@ func runTelemetryDisable(cmd *cobra.Command, _ []string) error { } func loadTelemetryConfig() (*config.Config, error) { - if configFile != "" { - cfg, err := config.LoadFromFile(configFile) - if err != nil { - return nil, err - } - if dataDir != "" { - cfg.DataDir = dataDir - } - return cfg, nil - } - cfg, err := config.Load() - if err != nil { - return nil, err - } - if dataDir != "" { - cfg.DataDir = dataDir - } - return cfg, nil + return loadCLIConfig(configFile) } // telemetryConfigSavePath returns the config path that telemetry subcommands diff --git a/cmd/mcpproxy/token_cmd.go b/cmd/mcpproxy/token_cmd.go index 97106caf..e324d024 100644 --- a/cmd/mcpproxy/token_cmd.go +++ b/cmd/mcpproxy/token_cmd.go @@ -128,31 +128,9 @@ Examples: } // loadTokenConfig loads the token command's config, honoring the --config and -// global --data-dir flags (GH #897, same class as #854). Without the DataDir -// override, socket.DetectSocketPath probes the default data dir and the CLI -// either reports "daemon is not reachable" or silently talks to the wrong -// daemon instance. +// global --data-dir flags (GH #897, same class as #854). func loadTokenConfig() (*config.Config, error) { - if tokenConfigPath != "" { - cfg, err := config.LoadFromFile(tokenConfigPath) - if err != nil { - return nil, err - } - // Respect global --data-dir flag - if dataDir != "" { - cfg.DataDir = dataDir - } - return cfg, nil - } - cfg, err := config.Load() - if err != nil { - return nil, err - } - // Respect global --data-dir flag - if dataDir != "" { - cfg.DataDir = dataDir - } - return cfg, nil + return loadCLIConfig(tokenConfigPath) } // newTokenCLIClient creates a cliclient.Client connected to the running MCPProxy. diff --git a/cmd/mcpproxy/trust_cert_cmd.go b/cmd/mcpproxy/trust_cert_cmd.go index 45e80bac..50efbd05 100644 --- a/cmd/mcpproxy/trust_cert_cmd.go +++ b/cmd/mcpproxy/trust_cert_cmd.go @@ -9,7 +9,6 @@ import ( "runtime" "strings" - "github.com/smart-mcp-proxy/mcpproxy-go/internal/config" "github.com/smart-mcp-proxy/mcpproxy-go/internal/tlslocal" "github.com/spf13/cobra" @@ -55,7 +54,7 @@ func init() { func runTrustCert(_ *cobra.Command, _ []string) error { // Load configuration to get certificate directory - cfg, err := config.LoadFromFile(configFile) + cfg, err := loadTrustCertConfig() if err != nil { return fmt.Errorf("failed to load configuration: %w", err) } diff --git a/cmd/mcpproxy/tui_cmd.go b/cmd/mcpproxy/tui_cmd.go index 56848326..82b4d7e9 100644 --- a/cmd/mcpproxy/tui_cmd.go +++ b/cmd/mcpproxy/tui_cmd.go @@ -34,10 +34,18 @@ func GetTUICommand() *cobra.Command { } defer func() { _ = logger.Sync() }() - // Load config to find daemon connection - cfg, err := config.Load() + // Load config to find daemon connection. An explicitly passed + // --config must fail loudly; only the implicit default-path load + // keeps the fall-back-to-defaults behavior. + cfg, err := loadTUIConfig() if err != nil { + if configFile != "" { + return fmt.Errorf("failed to load config: %w", err) + } cfg = config.DefaultConfig() + if dataDir != "" { + cfg.DataDir = dataDir + } } // Detect socket or fall back to TCP (probed with the API key) diff --git a/cmd/mcpproxy/upstream_cmd.go b/cmd/mcpproxy/upstream_cmd.go index 24fef636..b0ba4992 100644 --- a/cmd/mcpproxy/upstream_cmd.go +++ b/cmd/mcpproxy/upstream_cmd.go @@ -625,26 +625,7 @@ func outputError(err error, code string) error { } func loadUpstreamConfig() (*config.Config, error) { - if upstreamConfigPath != "" { - cfg, err := config.LoadFromFile(upstreamConfigPath) - if err != nil { - return nil, err - } - // Respect global --data-dir flag - if dataDir != "" { - cfg.DataDir = dataDir - } - return cfg, nil - } - cfg, err := config.Load() - if err != nil { - return nil, err - } - // Respect global --data-dir flag - if dataDir != "" { - cfg.DataDir = dataDir - } - return cfg, nil + return loadCLIConfig(upstreamConfigPath) } func createUpstreamLogger(level string) (*zap.Logger, error) {