Repository navigation
fix(webui): first-run call to action, onboarding path for a fresh setup, and "what needs me" on the landing page - #1049
Merged
Merged
Conversation
…ds me" on the landing page (F4) Web UI UX audit (docs/qa/ux-audit-webui-2026-08.md), first-run and onboarding. F3 — Servers page, zero servers. The page a new user lands on after closing the wizard said "No servers found / No servers available" and offered nothing to click, above four zero-valued stat tiles, four zero-count filter pills and a search box with nothing to search. It now renders the AgentTokens-style empty state: a headline, one line of why upstream servers matter, Add Server (primary), Browse Registry, and a quiet "Import from your AI client configs" link into the wizard's Servers step. The stat/filter/search chrome is hidden until a server exists, and "you have nothing yet" is now a separate state from "nothing matched your filter" — the two need different answers. F4 residual (after #1044) — #1044 routed /usage, made the analytics panel the default landing page and demoted the hub diagram, which is most of the fix. What it did not carry across is the other half the finding asked for: the landing page still answered "what happened" and not "what needs me". The servers-needing-attention and tools-pending-approval banners were inside the Overview panel, so a fresh landing could render charts while a server was down and an unreviewed tool was waiting, and say neither. They move above the panel switcher: visible on whichever panel is active, silent when there is nothing to act on. F19 — onboarding wizard, Servers step. It was entirely "pick which servers from your existing AI clients to import", so a user with no prior MCP config hit a dead end. Adds a nothing-to-import branch (browse the registry / add a server manually). Import & quarantine becomes the single primary and the unreviewed path becomes a link that says what it skips, "Import without review" — previously two similar primaries with the safer one rendered weaker. The Docker-isolation/quarantine panel is expanded by default and moved out of the sticky footer into the step body, where it can be open without squeezing the list; the import list is capped and scrolls in place instead of clipping its last row; and every step gets a Back.
Deploying mcpproxy-docs with
|
| Latest commit: |
2aa8cd3
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://0bc70279.mcpproxy-docs.pages.dev |
| Branch Preview URL: | https://fix-ux-first-run.mcpproxy-docs.pages.dev |
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Contributor
📦 Build ArtifactsWorkflow Run: View Run Available Artifacts
How to DownloadOption 1: GitHub Web UI (easiest)
Option 2: GitHub CLI gh run download 32844816328 --repo smart-mcp-proxy/mcpproxy-go
|
…d keep every step exitable Cross-model review (opencode/gpt-sol) on the first-run changes. P1 — the Servers page's import link sets the store flag and then routes to the Dashboard, which is the component that mounts the wizard. The wizard therefore came into existence with `show` already true and never saw a transition to watch: it rendered, but the open sequence never ran. The onMounted fallback that existed for this case kicked off the fetches but never chose a tab and never consumed the request — so the wizard opened on Clients regardless of what the opener asked for, and left the request behind to hijack the next plain open. One `immediate` watcher now owns opening; the fallback is gone. The requested tab is read before the first await, so it cannot outlive its own open or be consumed twice, and it applies immediately so the wizard never paints the wrong step while the fetches are in flight. P2 — `goToRegistry` did not await `dismiss()`. `dismiss()` awaits its mark-skipped calls before emitting `close`, so routing first tore down the Dashboard that owns the open flag and the emit could land with nothing left to clear it, springing the wizard back open on return. P2 — the Servers page fetches on neither mount nor route: App.vue and the Dashboard do. That leaves a window where `loading` is false and the list is still unknown, in which the filter/search empty state claimed "no servers match your search criteria" about a list nobody had fetched. Adds an explicit pending state, gated so servers already in hand are never held behind it. Also restores Close to the import action footer, which the review asked about and the live run then proved: a step whose only exits are "import" strands the user, and the committed e2e sweep dismisses an auto-opened wizard through exactly that control — on a fresh instance the wizard can land on this step. The security disclosure gains `group` so its arrow reflects the now-open state.
…h from hiding the server list Round 2 of the cross-model review (opencode/gpt-sol). P1 — nothing cancels the wizard's open sequence. A close, or a close followed by a reopen, while its five fetches were still in flight let the abandoned run finish and stamp its tab over the newer state — and call startPolling() on a wizard that was no longer open, leaving a 5s poll running until unmount. Each run now takes a generation token and checks it still owns the wizard before it writes; closing and unmounting both retire whatever is in flight. P2 — `loading.error` was write-only. Silent background refreshes set it and no success ever cleared it, so a single transient blip replaced the Servers page with a full-page "Failed to load servers" for the rest of the session, hiding a list we were still holding. A list that arrives now clears the error, and the blocking error state is reserved for the case with no fallback: when servers are already in hand, a failed refresh degrades to a non-blocking notice above the grid that says the data may be stale and offers Retry.
…lure path Round 3 of the cross-model review (opencode/gpt-sol). The open sequence's config read is the one fetch whose result the user can also edit: it drives the quarantine and Docker-isolation checkboxes, and this branch expands that panel by default, so toggling one the moment the step appears is now the expected path rather than a corner. A read left over from a superseded open landing afterwards would silently flip the checkbox back to the old server value. It now carries the open's generation token and does not commit if it has been superseded. The other fetches in the sequence write read-only display state that either run populates identically, so they are left alone. Server-list failures now take the same ticket as successes. Only the list itself was sequenced, so an older request failing after a newer one had already delivered a list would raise a stale error over fresh data — the mirror image of the stale-list problem the sequencing exists to prevent.
Round 4 of the cross-model review (opencode/gpt-sol). Only a delivered list advanced the mark, so a newer request that failed left it behind — and an older list arriving afterwards still passed the check, applying stale servers over the newer outcome and clearing its error on the way. Both outcomes are news about the same list, so both now claim the ticket.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Dumbris
added a commit
that referenced
this pull request
Aug 26, 2026
The document read as an open P0-P3 backlog, but seven PRs merged on 2026-08-25 (#1044, #1048, #1049, #1051, #1052, #1053, #1054) closed it. Anyone opening this file against current main was being handed 36 stale defects. Added a status banner and a Resolution section that separates three things honestly: - Re-checked live on v0.61.0 against a running isolated core with real /mcp traffic: F1, F4, F5, F7, F8, F12, F13, F20, F24, F26, F33 confirmed fixed, each with the observed evidence rather than a PR reference. - Still open: F6 is half fixed. Focus enters the Add Server dialog and submit is in a sticky footer, but Escape still does not close it - AddServerModal.vue has no key handler and JsonViewer.vue is the only component under components/ that handles Escape at all. F28's "key called optional" half is fixed; its three-surfaces state is not. - Not re-checked: named explicitly rather than implied, so the next reader knows which closures rest on a PR body and which on an observation. Five defects found on main while re-checking are recorded separately, since they are not regressions of anything in the audit. The first is security-relevant: quarantining a server through the config file leaves its tools in the search index, so retrieve_tools still hands the agent their names and descriptions - which is exactly what quarantine exists to prevent - before the call is refused. Runtime.QuarantineServer purges the index; the reload path detects the same transition (quarantined_changed: true) and does not. Also corrects the record on #1053: its body names F1-F16, which is the tray audit's numbering mis-pasted. #1055 is the tray PR; #1053 touches no Swift file.
Dumbris
added a commit
that referenced
this pull request
Aug 26, 2026
The document read as an open P0-P3 backlog, but eight PRs merged on 2026-08-25 (#1044, #1048, #1049, #1050, #1051, #1052, #1053, #1054) closed it. Anyone opening this file against current main was being handed 36 stale defects. Added a status banner and a Resolution section that separates four things: - Re-checked live on v0.61.0 against a running isolated core with real /mcp traffic: F1, F4, F5, F6, F7, F8, F12, F13, F20, F24, F26, F33 confirmed fixed, each with the observed evidence rather than a PR reference. - Not re-checked: named explicitly rather than implied, so the next reader knows which closures rest on a PR body and which on an observation. F28 is called out as partial - its "key called optional" half is fixed, its three-surfaces-at-once state is not. - Corrections to the audit itself: F23 mis-diagnosed a per-request structural estimate as a stale aggregation, and F31's "Manual badge has no tooltip" half was wrong even at audit time (spec-088 trust-mode badge, already titled). - Five defects found on main while re-checking, recorded separately since they are not regressions of anything in the audit. The first is security-relevant: quarantining a server through the config file leaves its tools in the search index, so retrieve_tools still hands the agent their names and descriptions - which is exactly what quarantine exists to prevent - before the call is refused. Runtime.QuarantineServer purges the index; the reload path detects the same transition (quarantined_changed: true) and does not. F6 carries a methodology note. Re-checking it produced a false negative: Escape driven through the browser-automation layer never reaches the page's document keydown listener, so the dialog looked stuck open. Dispatched in-page it closes at once, and #1050's useModalA11y is wired into all five <dialog :open> modals. The sweep assertion this document proposes must dispatch in-page, or it tests the automation layer instead of the app. Also corrects the record on #1053: its body names F1-F16, which is the tray audit's numbering mis-pasted. #1055 is the tray PR; #1053 touches no Swift file.
Dumbris
added a commit
that referenced
this pull request
Aug 26, 2026
* docs(qa): Web UI UX audit — ranked findings (ux-audit epic) Heuristic + live-instance audit of every routed Web UI view in light/dark, first-run and populated states, at 1440/820/390px. 36 findings ranked P0-P3 with view, evidence, heuristic violated, suggested fix and effort. Audited against standard heuristics plus the house activity-UI rules (grouping, failure-only status, intent reason, histogram/log parity). * docs(qa): record what the Web UI audit's backlog actually resolved to The document read as an open P0-P3 backlog, but eight PRs merged on 2026-08-25 (#1044, #1048, #1049, #1050, #1051, #1052, #1053, #1054) closed it. Anyone opening this file against current main was being handed 36 stale defects. Added a status banner and a Resolution section that separates four things: - Re-checked live on v0.61.0 against a running isolated core with real /mcp traffic: F1, F4, F5, F6, F7, F8, F12, F13, F20, F24, F26, F33 confirmed fixed, each with the observed evidence rather than a PR reference. - Not re-checked: named explicitly rather than implied, so the next reader knows which closures rest on a PR body and which on an observation. F28 is called out as partial - its "key called optional" half is fixed, its three-surfaces-at-once state is not. - Corrections to the audit itself: F23 mis-diagnosed a per-request structural estimate as a stale aggregation, and F31's "Manual badge has no tooltip" half was wrong even at audit time (spec-088 trust-mode badge, already titled). - Five defects found on main while re-checking, recorded separately since they are not regressions of anything in the audit. The first is security-relevant: quarantining a server through the config file leaves its tools in the search index, so retrieve_tools still hands the agent their names and descriptions - which is exactly what quarantine exists to prevent - before the call is refused. Runtime.QuarantineServer purges the index; the reload path detects the same transition (quarantined_changed: true) and does not. F6 carries a methodology note. Re-checking it produced a false negative: Escape driven through the browser-automation layer never reaches the page's document keydown listener, so the dialog looked stuck open. Dispatched in-page it closes at once, and #1050's useModalA11y is wired into all five <dialog :open> modals. The sweep assertion this document proposes must dispatch in-page, or it tests the automation layer instead of the app. Also corrects the record on #1053: its body names F1-F16, which is the tray audit's numbering mis-pasted. #1055 is the tray PR; #1053 touches no Swift file.
3 tasks done
Dumbris
added a commit
that referenced
this pull request
Sep 24, 2026
…ecks (#1358) Dashboard's usage-view first-run CTA used opacity-60 on inherited text color instead of the themed base-content token, producing low-contrast text against bg-base-200 in some themes. Switch to text-base-content/60. The web-ui-sweep servers-list and Activity/Usage checks assumed a non-empty fleet; a sweep run without a fixture upstream renders the first-run empty states instead of the KPI tiles and usage tile (#1049/#1216), which made those two checks fail. Anchor on whichever state the instance should show and assert accordingly instead of assuming a populated fleet.
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
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 three findings from the Web UI UX audit that sit on the first-run path — the one the ~48% one-and-done cohort walks.
F3 (P0) — first-run Servers page had no call to action
Servers.vuewith zero servers rendered "No servers found / No servers available" — a restatement, no button, no link — above four zero-valued stat tiles, four zero-count filter pills and a "Search servers…" box with nothing to search. This is the page a new user lands on after closing the setup wizard.It now follows the
AgentTokens.vuepattern the audit named as the model: a headline, one line of why upstream servers matter, and Add Server (primary) / Browse Registry, plus a quiet Import from your AI client configs link that opens the setup wizard on its Servers step.Two further changes fall out of it:
servers.length > 0rather than the load flag, so a fresh install never flashes a row of zeroes first.The first-run claim is gated on
serversStore.loaded(from #1044) — telling the user they have no servers is a claim, and only a list that actually arrived supports it.F4 (P0), residual after #1044
#1044 shipped most of this finding:
/usageis a real route, the analytics panel is the default landing page, and the hub diagram is demoted. What it did not carry across is the other half the finding asked for — the landing page answers "what happened" but not "what needs me".Both attention banners (servers needing attention, tools pending approval) were inside the Overview panel. Once Overview stopped being the default panel they went down with it, so a fresh landing could render charts while a server was down and an unreviewed tool was waiting, and say neither.
They move above the panel switcher: rendered on whichever panel is active, and silent when there is nothing to act on.
serversNeedingAttentionandloadPendingToolswere already driven on mount and on the 30s refresh regardless of panel, so this is a pure relocation.F19 (P2) — wizard had no path for a user with no existing MCP setup, and hid the security choice
Step 2 was entirely "pick which servers from your existing AI clients to import", so a user with no prior MCP config saw an empty list and no alternative — a dead end on the step that decides whether the product gets used.
btn-primary; Import as active becomes a link labelled Import without review with a tooltip saying what it skips. Previously two equally-weighted primaries with the safer one rendered as the weaker of the pair.<details>opens by default and moved out of the sticky footer into the scrollable step body. Expanded in the footer it ate the modal and squeezed the import list back down to the single clipped row the finding complains about; in the body it simply scrolls.max-h-[32vh]) instead of running off the bottom of the modal with no scrollbar affordance — and the cap leaves the security panel partly on screen, so the choice is visible rather than something to go looking for.stores/onboarding.tsgains a one-shotwizardInitialTabso the Servers page's import link can open the wizard on the right step; it is consumed on open, so a later plainopenWizard()is never silently redirected.Cross-model review (opencode / gpt-sol)
Four rounds, verdict CLEAN on the fifth. Every finding was reproduced before being fixed, and each fix ships with a test that fails against the pre-fix code.
Round 1
showalready true and never saw a transition to watch; theonMountedfallback for that case started the fetches but never picked a tab or consumed the request, so it opened on Clients and leaked the request into the next open. One{ immediate: true }watcher now owns opening and the fallback is gone.goToRegistrydid not awaitdismiss(), which awaits its mark-skipped calls before emittingclose; routing first tore down the Dashboard that owns the flag.Round 2
loading.errorwas write-only: silent refreshes set it, nothing cleared it, so one transient blip replaced the Servers page with a full-page error for the rest of the session while a usable list sat behind it.Round 3
Round 4
Not fixed here
F36's "
MCPPROXY active / just startedreads as a stuck state" is left alone deliberately. The diagnosis:uptimeinDashboard.vueis derived fromserverFirstSeen, which is when this browser tab first saw the server running — so it resets to "just started" on every page load no matter how long the core has been up — and the computed readsDate.now()with no reactive time source, so it does not tick. Fixing it properly means adding a real start time toGET /api/v1/status(the value exists only as a Prometheusmcpproxy_uptime_secondsmetric today), which pulls in a Go change, both-edition builds, swagger and generated-contracts regen. That does not belong in a frontend-only PR, and the panel it sits on is no longer the landing page after #1044.Verification
vitest— 733 passed / 72 files, including 37 new assertions across three new specs (servers-first-run-empty,onboarding-wizard-servers-step,dashboard-attention-banners) plus additions toservers-store.vue-tsc --noEmitclean.make build(embedded frontend): a first-run core on an empty scratch config (the state under test) and a populated one (3 servers, one unreachable, one disabled) for regressions — 14/14, plus a 390px pass and a no-horizontal-scroll assertion.e2e/web-ui-sweepsuite green against the populated instance.