fix: first-run user test findings in setup import, secrets, status pill and profile Try it (Spec fix-usertest-web) - #1473
Merged
Conversation
…ls unsaved edits (T153)
…ng (T176, T177, T154)
…t 1100-1279px while servers await review
Deploying mcpproxy-docs with
|
| Latest commit: |
2df8fd0
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://31f4d1fb.mcpproxy-docs.pages.dev |
| Branch Preview URL: | https://fix-usertest-web.mcpproxy-docs.pages.dev |
F2.1: ImportServers gains a show-message prop; the wizard suppresses the importer's own imported line while its servers-import-done card shows, so the Servers step has one completion state. F3.1: ManualServerForm env/header rows get a stable id used as the v-for key, so removing an earlier row no longer hands its SecretToggle (and its revealed flag) to the next row.
Contributor
📦 Build ArtifactsWorkflow Run: View Run Available Artifacts
How to DownloadOption 1: GitHub Web UI (easiest)
Option 2: GitHub CLI gh run download 37070904376 --repo smart-mcp-proxy/mcpproxy-go
|
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
11 tasks
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
github-actions Bot
pushed a commit
that referenced
this pull request
Oct 3, 2026
…le reach (#1482) Docs, changelog and verified fixes from a first-run, upgrade and user-test pass on `main` (335477e). ## Docs - `docs/features/profiles.md`: profiles are optional. Without an effective profile, no profile-level restriction applies (a scoped credential's own grant still does). The quick start is presented as being for scoped access. - `docs/configuration.md` (`quarantined` row): explains the real default. A server added through the UI, CLI or API follows its trust mode. A first-seen config-file server with no `quarantined` key and no `config.db` record is held for review. An explicit value wins, and a recorded server keeps its state. Links to the admission rules. - `docs/features/security-quarantine.md`: the hand-editing section names the extra conditions: quarantine is enabled, and the trust mode is not `auto` (including legacy `auto_approve_tool_changes`/`skip_quarantine` resolving to `auto`), matching `ServerConfig.EffectiveTrustMode`. - `docs/getting-started/quick-start.mdx`: a plain note that a server added by editing the file is held for review until approved. - `CHANGELOG.md`: the security, review, catalog, telemetry, CLI and first-run fixes merged since v0.69.0 (#1463, #1467–#1473, #1481) plus this PR's. ## Fixes (each reproduced in an isolated instance, regression test fails on main) - **Upgrade start advisory repeated.** `reportPreFixAdmissions` ran on every gate pass (initial gate, `LoadConfiguredServers`, every publish), so an upgrade start logged the "predate the config-load admission gate" advisory 2–5 times. It is now reported once per server per process. Gate decisions are unchanged. - **Benign startup race logged as ERROR.** Both the supervisor reconcile and `LoadConfiguredServers` → `AddServer` asked a not-yet-ready client to connect. The second call was correctly refused with "connection already in progress or established (state: Connecting)" but logged at ERROR, and server identity registration was skipped. A new `managed.ErrConnectAlreadyActive` sentinel (same message text) lets `AddServer` treat it as success at debug level. - **`set_profile` under-reported a switchable client's reach.** After a successful switch, `resolveEffectiveProfileForJustSetSlug` treated the client binding as authoritative and reported the binding's servers instead of the selected profile's. ## Verification - Cross-model review: codex gpt-6.1-sol, chunked. Three low findings: two doc-wording findings are applied, and the third (the per-process advisory set is never cleared) is a deliberate trade-off. - Live re-verification on isolated instances: each symptom was reproduced on a main build and is gone on this branch. The upgrade case was run over a v0.52.1 data dir.
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.
Summary
Fixes four findings from a first-run user test of
main(findings F-04, F-05, F-07 and the first-load status journey). They are recorded as tasks in Spec 109 Phase 17 (T172 to T177) and Spec 108 Phase 17 (T153 to T154), with decisions in research D37 (Spec 109) and D40 (Spec 108). No new dependencies, no MCP or CLI golden changes, no generated contract types changed.Specs:
specs/109-ux-navigation-consistency/(tasks.md Phase 17, research.md D37, quickstart.md, contracts/navigation-map.md and rest-api.md) andspecs/108-profiles-v3/(tasks.md Phase 17, research.md D40).User-visible changes
REST (F-04, T172)
POST /api/v1/servers/import/path?preview=trueof a client config that exists but holds no MCP servers (0 bytes, whitespace,{}, no or empty server map) now answers200with an emptyimportedlist and the format from the hint, instead of400. Previously the setup wizard and the detected importer each logged a 400 per empty Claude Code or Claude Desktop file.preview=false) of such a file, a malformed file, the multipart upload and/import/jsonstill answer400.configimport.ErrTypeNoServers,configimport.IsNoServers,httpapi.emptyImportPreview. Swagger description updated.Web UI
0/4and carries the full text in its title; between 1100 and 1279 px the routing-mode chip and offline count are hidden while servers await review so the header does not overflow. Before the first server list it reads "Loading servers…", and "Servers unavailable" if that load failed.server:toolplus description (it printed[object Object]because the panel read a flat item while the API returns{score, tool: {...}}), and says whether it uses your unsaved edits. While the draft differs from the saved profile, the tool counts read "Saved profile: 3 visible · 6 hidden" with a note pointing to Try it.Not changed
CLI, MCP tools, macOS app (macOS masking and a draft-evaluated effective-tools route are follow-ups), and the files owned by other PRs (
ReviewScreen.vue,CatalogSearch.vue,internal/registries/**).Tests added
TestImportFromPath_PreviewEmptyClientConfigIsEmptyNot400,TestImportFromPath_ApplyEmptyClientConfigStill400,TestImportFromPath_PreviewMalformedJSONStill400,TestImportServersJSON_EmptyMcpServersStill400,TestIsNoServers.onboarding-servers-step-state,import-servers-completion,onboarding-wizard-import-completion,secret-toggle-masking,status-pill-awaiting-review,profile-try-unsaved; extendedadd-server-manual,add-server-paste,profile-policy-editor(real nested hit shape) andstatus-pill(loaded = truefixture line).e2e/web-ui-sweep/usertest-web-fixes.spec.tsat 1440x900 and 900x900 (added toscripts/run-web-smoke.sh); the pill regex innavigation-consistency.spec.tsnow accepts the awaiting-review wording.acceptance-index.json(US5-5, US6-2, US7-4) andparity-matrix.json(Spec 109 row 17, Spec 108 row 6) reference the new tests.