fix(cli): activity/tui/credential/security honor global --data-dir; extract shared loadCLIConfig#909
Merged
Merged
Conversation
…xtract shared loadCLIConfig (#908) Bare config.Load() in activity (3 sites), tui, credential, and the security client ignored the global --data-dir flag, so daemon detection (socket.DetectSocketPath) probed the default data dir — same class as #854/#897/#907. connect/feedback/registry/security/status/telemetry already carried the override; the issue's wider file list was stale. Extract the repeated load-then-override pattern into loadCLIConfig and collapse the pattern-identical loaders (doctor, token, upstream, status, telemetry, feedback, connect) onto it. auth/call/code/tools keep their distinct home-dir/stat semantics; registry keeps its DefaultConfig fallback. TestLoadersHonorGlobalDataDirFlag now covers every command loader (16 rows) so nothing can drift again.
Deploying mcpproxy-docs with
|
| Latest commit: |
db008a4
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://1765b217.mcpproxy-docs.pages.dev |
| Branch Preview URL: | https://fix-908-cli-datadir-remainin.mcpproxy-docs.pages.dev |
|
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
📦 Build ArtifactsWorkflow Run: View Run Available Artifacts
How to DownloadOption 1: GitHub Web UI (easiest)
Option 2: GitHub CLI gh run download 30085651231 --repo smart-mcp-proxy/mcpproxy-go
|
credential_cmd.go is //go:build server, so the personal-edition build and lint missed that loadCredentialConfig() removed the file's last config. usage; only the CI Server Edition job caught it.
…n bad explicit --config (Codex round 1) trust-cert used bare config.LoadFromFile(configFile), which with no -c flag never read the default config file at all — so it ignored both --data-dir and tls.certs_dir from ~/.mcpproxy/mcp_config.json when resolving the certs dir. Route it through loadCLIConfig (test row 17). tui now propagates a load error when --config was explicitly passed instead of silently falling back to defaults; the implicit default-path load keeps the fallback.
… (Codex round 2) resolveCredentialBaseURL silently fell back to http://127.0.0.1:8080 when an explicitly passed --config failed to load; now it returns the error (implicit default-path load keeps the fallback), mirroring the tui fix. newCredentialClient and the four run functions propagate it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #908.
What was actually broken
The issue's file list was partially stale — an audit of every
config.Load()site showedconnect,feedback,registry,security,status, andtelemetryalready applied the--data-diroverride. The genuinely broken sites were:activity_cmd.go— 3 inline loads (client, watch, export) with no override, somcpproxy activity ... --data-dir Dresolved the socket against the default data dirtui_cmd.go— bareconfig.Load()feedingcfg.DataDirintosocket.DetectSocketPath, somcpproxy tui --data-dir Dtargeted the wrong daemon (same user-visible bug as token CLI ignores global --data-dir for daemon socket detection (missed by #854) #897)credential_cmd.go— bareconfig.Load()inresolveCredentialBaseURL(Listen-only today, fixed for consistency)Changes
cmd/mcpproxy/cli_config.go: sharedloadCLIConfig(explicitPath)— the "finally extract one shared loader" option from the issue — plus thin per-command loaders for activity/credential/security/tuiauth/call/code/toolskeep their distinct home-dir-default + stat-check semantics;registrykeeps its DefaultConfig fallbacktuiDefaultConfig fallback path now also applies--data-dirTestLoadersHonorGlobalDataDirFlagextended from 6 to 16 rows — every command loader is now covered, so no command can silently drift back to a bare loadVerification
go test ./cmd/mcpproxy/— all pass (new rows watched failing first: 4 undefined loaders)golangci-lint run --config .github/.golangci.yml ./cmd/mcpproxy/...— 0 issues./scripts/test-api-e2e.sh— 64 passed, 0 failed, 1 pre-existing skip