Repository navigation
Desktop app: tell users the MCP server exists, and how to wire it up - #423
Merged
Merged
Conversation
The desktop app had no MCP integration at all -- no reference to it anywhere under apps/netscli-gui. Someone who only ever opens the app had no way to learn that netscli ships an MCP server, let alone connect one to an agent. WHAT THIS IS NOT. Issue #412 proposed install/uninstall buttons over `netscli mcp-service`. Three things in the code contradict that, and the panel does none of it: - mcp_service.rs has zero cfg(target_os) and writes a systemd user unit to ~/.config/systemd/user/. On Windows it creates C:\Users\<user>\.config\systemd\user\netscli-mcp.service and prints "Service file created" -- a false success on the desktop app's main platform. - The issue says both need elevation. They do not; `systemctl --user` is a user service. - The unit runs `netscli serve` with Restart=always. serve is stdio-only, and systemd's default StandardInput=null hands it /dev/null, so it reads EOF on its first read and exits 0 (server.rs: `Ok(0) => break`). Restart brings it back five seconds later to do the same thing, forever. The issue argues against a start-server button for exactly this reason and then proposes wiring up a service with the same flaw. Those are left alone here and want their own change. WHAT THE PANEL DOES. The thing that actually solves discovery is the config block, and it needs a binary to point at -- so the panel detects one first. The detection is the part worth reading. This crate links netscli-core directly, tauri.conf.json has no externalBin, and nothing here spawns a netscli process, so the desktop app does NOT ship the CLI. Someone who ran `winget install netscli-gui` or opened the MSI has no netscli binary at all, and for them the honest answer is "install the CLI", not a config naming a file that does not exist. Both states are therefore first-class: found renders the config, missing renders one install command and a link to the docs page carrying the rest. The absolute path, not the bare name, because an MCP client launched from the desktop session does not necessarily inherit a shell's PATH -- `"command": "netscli"` fails for a client that a terminal would have resolved fine. That is also why detection checks PATH first and then the documented install locations per platform. `--version` is run on a candidate before accepting it. Finding a file named netscli is not finding this program, and the cost of being wrong is paid far from here: the block is pasted into another application and fails there, silently, later. The probe is bounded at 3s so a hung candidate cannot freeze the dialog. The block is built with JSON.stringify rather than a template literal. A Windows path is full of backslashes and has to reach the client's config file escaped; hand-writing that is how it gets missed on the one platform most desktop users are on. Verified by parsing the rendered block back and comparing to the input path. settings-mcp.css is a new file rather than more of settings.css, which was at 257 lines and would have gone over the 300-line guard. Same reason settings-controls.css and settings-number-field.css are separate. It also overrides one inherited rule: settings.css clips `small` to a single line with an ellipsis, which suits a one-line note beside a control and cut this section's explanation to "... reopen Settings t…" -- losing exactly the half that says what to do. Caught by looking at the rendered panel, not the diff. Verified: both states rendered and read in the running app; the config parses as JSON and round-trips to the original Windows path; detection run for real on this machine returned a path, version 0.3.1 and os=windows. astro-side untouched. cargo fmt clean, clippy -D warnings clean, 3 Rust tests, 200 frontend tests, file-size guard clean. The Selenium render harness would not run to completion on this machine -- it exits 0 straight after the build without reaching the driver, twice -- so the new assertion in it is unproven locally and will first execute on windows-latest in the GUI Render workflow.
The check resolved a netscli.com link by looking for `<slug>.astro` or `<slug>/index.astro` under site/src/pages. That was true when it was written and is not now: src/pages also holds endpoint routes that return a file rather than a page -- install.sh.ts, install.ps1.ts, llms.txt.ts and llms-full.txt.ts. So it reported the site's OWN canonical install one-liner, https://netscli.com/install.sh, as a link the site does not build. The route exists (site/src/pages/install.sh.ts), the site serves it, and the site's install-urls.ts generates that exact URL for the hero and the install panel. Found by the MCP settings panel, which is the first thing in the app to link to it. The link was correct; the gate could not see the route. `.ts` is added alongside `.astro` for both the bare and index forms, so the other three endpoint routes are covered too rather than only the one that happened to fail.
The GUI Render workflow last succeeded on 2026-07-14. Every run since is a
failure or a cancellation -- 74 and 17 of them -- so the gate has reported
nothing for two months. Three separate bugs, each of which alone was enough
to stop it.
PORT RACE (why the driver never started). main() called getFreePort() at the
top, then handed that number to msedgedriver minutes later, after the Rust
build and after the app launched. getFreePort reserves nothing: it binds an
ephemeral port, reads the number and closes, so the result is only a fact
about the instant it was taken. WebView2 starts a swarm of processes that
take ephemeral ports, and one of them took that one. msedgedriver then
exited 1 after printing only its banner:
[SEVERE]: bind() returned an error: Only one usage of each socket
address (protocol/network address/port) is normally permitted. (0x2740)
IPv6 port not available. Exiting...
Reproduced by holding the port open and spawning the driver on it, which
gives that exact output and exit code. The port is now chosen inside
startNativeDriver at the moment it spawns, with two retries on a bind
collision. An explicit TAURI_DRIVER_PORT is honoured and never retried: if a
port was named, failing to get it is the answer.
THE 21-MINUTE HANG (why CI burned its whole 30-minute budget). On failure
launchApplication let the rejection escape with the app still running, and
the app is spawned with piped stdio, so those pipes kept node's event loop
alive. The caller cannot clean that up -- `appProcess` is only assigned from
this function's return value, so on the throw path it is still undefined and
stopProcess(appProcess) is a no-op. CI printed "Timed out waiting for port
64892" at 09:21 and sat there until 09:42. It now kills its own child before
rethrowing, so the failure is immediate and legible.
TOOLTIP RACE (what failed once the first two were fixed). The assertion ran
dispatch, then waitForText, then a separate executeScript to measure.
AppTooltip hides 40ms after a pointerout, and every WebDriver round trip
costs far more than that, so the wait passed and the measurement then found
nothing. It reported "Global tooltip should render", which reads as a broken
tooltip rather than a test that looked too late.
Hovering and measuring now happen in one in-page script, with the hover
re-asserted every animation frame so the hide timer is continually cancelled
whatever triggers it, and the measurement taken in the same tick as the
match. A real failure still fails, and now reports what the tooltip held
instead of only that it was absent. dispatchTooltipPointerOver has no callers
left and is deleted rather than kept warm.
PROGRESS OUTPUT. The harness printed nothing between the build and the first
scenario, so a stall in setup was indistinguishable from a clean exit: the
local symptom was "builds, then exits 0" and diagnosing it needed
instrumentation added by hand, twice. Each setup stage now says what it is
doing. That is what turned this from unreproducible into three named bugs.
Verified: four consecutive local runs exit 0 and write every scenario
screenshot -- scan, dns, inspect, interfaces, sweep, menus/toolbar, dark,
light, narrow. Run four times specifically because two of these were races
and one green run proves little. eslint clean, file-size guard clean.
fstubner
added a commit
that referenced
this pull request
Sep 21, 2026
Twenty-eight non-dependency commits have landed since 0.3.1 and four of them were in [Unreleased]. Cutting a release on that would have shipped notes describing a fraction of it. Added: discover naming hosts from mDNS (#418), and the desktop app's MCP panel (#423). Fixed: result tables being unselectable (#417), and the changelog page losing its paragraph breaks (#426). Changed: one bullet for the site and docs pass, following the shape 0.3.1 used rather than listing twelve site PRs a reader does not care about individually. Security: the three advisories cleared together in #428. The release pipeline fixes, the CI changes and the site deploy fix are deliberately absent. #411 dropped the internal-changes section on purpose and this follows it. The rustls advisory is described as having shipped, because it did: 0.23.40 is in v0.3.1's Cargo.lock. The other two are the website's build dependencies and reach no release artifact, which the entry now says rather than lumping all three together. Also repairs the header entry, which carried a duplicated paragraph from an earlier edit.
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
1 participant
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.
Closes #412.
The desktop app had no MCP integration at all. Someone who only ever opens the app had no way to learn that netscli ships an MCP server, let alone connect one to an agent.
What this deliberately does not do
#412 proposed install/uninstall buttons over
netscli mcp-service. Three things in the code contradict that, so the panel does none of it:mcp_service.rshas zerocfg(target_os)and writes a systemd unit. On Windows it createsC:\Users\<user>\.config\systemd\user\netscli-mcp.serviceand prints "✅ Service file created" — a false success on the desktop app's main platform.systemctl --useris a user service.netscli servewithRestart=always, systemd hands it/dev/nullon stdin,servehitsOk(0) => breakon its first read and exits 0 — then restarts 5 seconds later, forever.None of that is touched here; it wants its own change.
What the panel does
The thing that actually solves discovery is the config block, and it needs a binary to point at — so it detects one first.
The desktop app does not ship the CLI. This crate links
netscli-coredirectly,tauri.conf.jsonhas noexternalBin, and nothing insrc-taurispawns a netscli process. Someone who ranwinget install netscli-guior opened the MSI has no netscli binary at all. So both states are first-class:Absolute path, not the bare name. An MCP client launched from the desktop session doesn't necessarily inherit a shell's PATH, so
"command": "netscli"fails for a client a terminal would have resolved fine. Detection checks PATH first, then the documented install locations per platform.--versionis run before a candidate is accepted. Finding a file named netscli isn't finding this program, and the cost of being wrong is paid far away — the block is pasted into another application and fails there, silently, later. Bounded at 3s so a hung candidate can't freeze the dialog.JSON.stringify, not a template literal. A Windows path is full of backslashes and has to reach the client's config escaped. Verified by parsing the rendered block back and comparing to the input path.Verified
0.3.1,os=windows.cargo fmtclean,clippy -D warningsclean, 3 Rust tests, 200 frontend tests, file-size guard clean.One defect this caught:
settings.cssclipssmallto a single line with an ellipsis, which suits a one-line note beside a control and cut this section's explanation to…reopen Settings t…— losing exactly the half that says what to do. Found by looking at the rendered panel, not the diff.Not proven locally
The Selenium render harness would not run to completion on this machine — it exits 0 straight after the build without reaching the driver, twice. The new
assertMcpSectionassertion in it is therefore unproven locally and will first execute onwindows-latestin the GUI Render workflow.settings-mcp.cssis a new file rather than more ofsettings.css, which was at 257 lines and would have crossed the 300-line guard — same reasonsettings-controls.cssandsettings-number-field.cssare separate.