fix(cli): token commands honor --data-dir; unwrap REST envelope in table output#907
Merged
Conversation
…ble output (#897) Two token-CLI fixes: 1. GH #897: newTokenCLIClient loaded config with bare config.Load(), ignoring the global --data-dir flag — the same class of bug #854 fixed for doctor/auth/call/code/tools. With no default daemon this reports "daemon is not reachable"; with one running it silently talks to the WRONG daemon (verified locally: token create --data-dir /tmp/q897 minted the token on ~/.mcpproxy's daemon). Fix mirrors the #854 loader idiom: new loadTokenConfig() honoring --config (new persistent flag on the token command) and the global --data-dir, used by newTokenCLIClient. Regression coverage: one new row in TestLoadersHonorGlobalDataDirFlag. 2. Found verifying #897: the REST API wraps token responses in the standard {"success":true,"data":{...}} envelope, but the CLI table paths read top-level keys — so `token list` ALWAYS printed "No agent tokens configured" and `token create`/`regenerate` never displayed the minted token (which is shown only once by design; the only escape was -o json on the raw envelope). New parseTokenAPIResponse() unwraps the envelope (legacy bare shape passes through) for all four parsing handlers; -o json now prints the bare payload, consistent with the other CLI commands. Fixes #897 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Deploying mcpproxy-docs with
|
| Latest commit: |
9f17f94
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://6766ccb8.mcpproxy-docs.pages.dev |
| Branch Preview URL: | https://fix-897-token-datadir.mcpproxy-docs.pages.dev |
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
- Handler-level tests drive the real runTokenList/runTokenCreate against an httptest daemon (MCPPROXY_TRAY_ENDPOINT seam) with responses built by contracts.NewSuccessResponse — reverting any parseTokenAPIResponse call site or renaming an envelope field now fails tests. - config.Load() branch of loadTokenConfig (the exact GH #897 user flow, no --config) covered via HOME-sandboxed test; deleting the --data-dir override there no longer passes the suite. - New --config/-c persistent flag registration asserted. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
📦 Build ArtifactsWorkflow Run: View Run Available Artifacts
How to DownloadOption 1: GitHub Web UI (easiest)
Option 2: GitHub CLI gh run download 30072395901 --repo smart-mcp-proxy/mcpproxy-go
|
Dumbris
added a commit
that referenced
this pull request
Jul 24, 2026
…xtract shared loadCLIConfig (#909) * fix(cli): activity/tui/credential/security honor global --data-dir; extract 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. * fix(cli): drop unused config import in server-only credential_cmd.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. * fix(cli): trust-cert honors --data-dir + default config; tui errors on 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. * fix(cli): credential commands propagate explicit --config load errors (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 #897
Fix 1 —
--data-dir(the reported bug)newTokenCLIClientused bareconfig.Load(), ignoring the global--data-dirflag — the exact class #854 fixed fordoctor/auth/call/code/tools;tokenwas missed. Verified locally that this is worse than the reported "daemon is not reachable": with a default daemon running,token create --data-dir /tmp/q897silently minted the token on the wrong daemon (~/.mcpproxy).Fix mirrors the #854 idiom:
loadTokenConfig()honoring a new--configpersistent flag + the global--data-dir, one new row inTestLoadersHonorGlobalDataDirFlag(covers all six subcommands since they share the loader).Fix 2 — REST envelope (found during verification)
All token table outputs parsed top-level keys, but the API wraps responses in
{"success":true,"data":{...}}. Result:token listalways printed "No agent tokens configured", andtoken create/regeneratenever displayed the minted token — which is shown only once by design. NewparseTokenAPIResponse()unwraps the envelope (legacy bare shape passes through);-o jsonnow prints the bare payload, consistent withupstream listet al.Local verification
~/.mcpproxy) with--data-dir /tmp/q897; after fix: token lands on the scratch daemon, default untouched.createshows token,listtable populated,show,revoke,regenerateshows new token,delete) — all honoring--data-dir../scripts/test-api-e2e.sh: 65/65 passed.go test -race ./cmd/mcpproxy/: ok. golangci-lint v2: 0 issues.Not in scope (same class, filed as follow-up)
Bare
config.Load()without the--data-diroverride also remains inactivity,connect,credential,feedback,registry,security,telemetry,tui,statuscommands —tuiandstatusalso feedDetectSocketPath. Will file a follow-up issue.🤖 Generated with Claude Code