From f34c85f80844538354c06f71908e34f4c4ce0fbd Mon Sep 17 00:00:00 2001 From: Felix Stubner Date: Thu, 17 Sep 2026 08:20:58 +0100 Subject: [PATCH 1/3] Tell desktop users the MCP server exists, and how to wire it up MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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\\.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. --- .../scenarios/helpers/settingsDialog.mjs | 49 ++++- .../netscli-gui/src-tauri/src/commands/mcp.rs | 205 ++++++++++++++++++ .../netscli-gui/src-tauri/src/commands/mod.rs | 2 + apps/netscli-gui/src-tauri/src/main.rs | 7 +- .../src/components/shell/McpServerSection.tsx | 153 +++++++++++++ .../src/components/shell/SettingsDialog.tsx | 3 + apps/netscli-gui/src/services/netscli.ts | 14 ++ apps/netscli-gui/src/styles/shell.css | 1 + .../src/styles/shell/settings-mcp.css | 77 +++++++ 9 files changed, 507 insertions(+), 4 deletions(-) create mode 100644 apps/netscli-gui/src-tauri/src/commands/mcp.rs create mode 100644 apps/netscli-gui/src/components/shell/McpServerSection.tsx create mode 100644 apps/netscli-gui/src/styles/shell/settings-mcp.css diff --git a/apps/netscli-gui/e2e/tauri-render/scenarios/helpers/settingsDialog.mjs b/apps/netscli-gui/e2e/tauri-render/scenarios/helpers/settingsDialog.mjs index 9ca2f79e..3bec4841 100644 --- a/apps/netscli-gui/e2e/tauri-render/scenarios/helpers/settingsDialog.mjs +++ b/apps/netscli-gui/e2e/tauri-render/scenarios/helpers/settingsDialog.mjs @@ -77,6 +77,7 @@ async function assertSettingsDialog(driver) { await waitForText(driver, '.settings-interface-list', /\S/); await waitForText(driver, '.settings-interface-list', /Primary/i); await assertInterfacePickerRows(driver); + await assertMcpSection(driver); await closeSettingsDialog(driver); } @@ -137,6 +138,52 @@ async function assertInterfacePickerRows(driver) { } } +/** + * The MCP section, in whichever state this machine puts it in. + * + * Both states are legitimate and which one appears depends on the machine, so + * the assertion is on the shape of what is shown rather than on which branch + * rendered. The desktop app does not ship the CLI, so "not installed" is the + * common case for a real user and must not read as a failure here either. + * + * The config assertion is the one worth having: the block is pasted into + * another application and fails there, silently, so a malformed one is + * expensive and invisible. Parsing it catches the failure this is most likely + * to have -- an unescaped Windows path, which is a string full of backslashes + * going into JSON. + */ +async function assertMcpSection(driver) { + const mcp = await driver.executeScript(` + const section = document.querySelector('[data-testid="settings-mcp-section"]'); + return { + present: section !== null, + text: section?.innerText ?? '', + config: document.querySelector('[data-testid="settings-mcp-config"]')?.textContent ?? null, + install: document.querySelector('[data-testid="settings-mcp-install"]')?.textContent ?? null, + }; + `); + + assert.ok(mcp.present, 'Settings should carry an MCP Server section'); + assert.ok( + mcp.config !== null || mcp.install !== null, + 'MCP section should show either a client config or an install command, got neither', + ); + + if (mcp.config === null) { + assert.match(mcp.install, /netscli/, 'Install command should mention netscli'); + return; + } + + const parsed = JSON.parse(mcp.config); + const entry = parsed?.mcpServers?.netscli; + assert.ok(entry, 'Config should define an mcpServers.netscli entry'); + assert.deepEqual(entry.args, ['serve'], 'The client starts the server with `serve`'); + assert.ok( + entry.command.length > 0 && entry.command !== 'netscli', + `Config should carry an absolute path, not a bare name, got "${entry.command}"`, + ); +} + async function openSettingsDialog(driver) { const open = await driver.executeScript( "return document.querySelector('[data-testid=\"settings-dialog\"]') !== null", @@ -186,4 +233,4 @@ async function assertSettingsDialogCentring(driver) { ); } -export { assertSettingsDialog, assertSettingsDialogCentring, closeSettingsDialog, openSettingsDialog }; +export { assertMcpSection, assertSettingsDialog, assertSettingsDialogCentring, closeSettingsDialog, openSettingsDialog }; diff --git a/apps/netscli-gui/src-tauri/src/commands/mcp.rs b/apps/netscli-gui/src-tauri/src/commands/mcp.rs new file mode 100644 index 00000000..11fb2a6f --- /dev/null +++ b/apps/netscli-gui/src-tauri/src/commands/mcp.rs @@ -0,0 +1,205 @@ +//! Locating the `netscli` CLI, for the MCP section of Settings. +//! +//! WHY THIS EXISTS AT ALL. The desktop app does not ship the CLI and does not +//! depend on it: this crate links `netscli-core` directly, there is no +//! `externalBin` in tauri.conf.json, and nothing here spawns a `netscli` +//! process. So someone who installed only the desktop app -- `winget install +//! netscli-gui`, the MSI, the DMG, the .deb, the AppImage -- has no `netscli` +//! executable anywhere on the machine. +//! +//! That matters because the MCP server IS the CLI. An MCP client starts the +//! server itself by running `netscli serve` and talking JSON-RPC over the +//! pipe, so the config block a client needs is a path to that binary. Without +//! one there is nothing to configure, and the honest thing for the panel to +//! say is "install the CLI first" rather than to print a config naming a file +//! that does not exist. +//! +//! WHY `--version` IS RUN. Finding a file called `netscli` is not the same as +//! finding this program. A config block is pasted into another application and +//! fails there, silently, hours later -- the cost of being wrong is paid far +//! from here, so the check is worth one process spawn. The binary is only +//! accepted if it answers `--version` and names itself. + +use serde::Serialize; +use std::path::{Path, PathBuf}; +use std::time::Duration; +use tokio::process::Command; +use tokio::time::timeout; + +/// The filename to look for. Windows only ever ships the `.exe`; PATHEXT +/// resolution is deliberately not reimplemented here. +const BIN_NAME: &str = if cfg!(windows) { + "netscli.exe" +} else { + "netscli" +}; + +/// A found binary gets this long to answer `--version`. +/// +/// Bounded because this runs while the Settings dialog is opening and the +/// candidate is an arbitrary executable that happens to carry the right name. +/// A hung probe would freeze the panel with no way out. +const PROBE_TIMEOUT: Duration = Duration::from_secs(3); + +#[derive(Serialize)] +#[serde(rename_all = "camelCase")] +pub(crate) struct CliDetection { + /// Absolute path to a verified `netscli`, or `None` when the CLI is not + /// installed. The frontend switches the whole section on this. + path: Option, + /// What the binary reported, e.g. `0.3.1`. Shown so a stale CLI beside a + /// newer desktop app is visible rather than merely present. + version: Option, + /// Which platform's install route the panel should offer when `path` is + /// empty. Decided here because this crate already knows at compile time, + /// and the alternative in the webview is sniffing a user-agent string. + /// The wording that goes with it stays in the frontend with the rest of + /// the UI copy. + os: &'static str, +} + +/// `windows` | `macos` | `linux`, matching the site's platform keys so the two +/// sets of install instructions can be compared against each other. +const fn current_os() -> &'static str { + if cfg!(windows) { + "windows" + } else if cfg!(target_os = "macos") { + "macos" + } else { + "linux" + } +} + +/// Where a `netscli` might live, in the order worth trying. +/// +/// PATH comes first because it is the one the user's own shell would pick, +/// and because an MCP client that DOES inherit PATH will resolve the same +/// file. The explicit directories after it are the install routes the site +/// documents, and they exist because GUI-launched clients frequently do not +/// inherit a login shell's PATH -- the reason the config block wants an +/// absolute path in the first place. +fn candidate_paths() -> Vec { + let mut out = Vec::new(); + + if let Some(path) = std::env::var_os("PATH") { + out.extend(std::env::split_paths(&path).map(|dir| dir.join(BIN_NAME))); + } + + if let Some(home) = dirs::home_dir() { + // `cargo install netscli`, on every platform. + out.push(home.join(".cargo").join("bin").join(BIN_NAME)); + + #[cfg(windows)] + out.push(home.join("scoop").join("shims").join(BIN_NAME)); + + #[cfg(not(windows))] + out.push(home.join(".local").join("bin").join(BIN_NAME)); + } + + #[cfg(windows)] + if let Some(local) = dirs::data_local_dir() { + // Where winget puts its shims. + out.push( + local + .join("Microsoft") + .join("WinGet") + .join("Links") + .join(BIN_NAME), + ); + } + + #[cfg(not(windows))] + { + out.push(PathBuf::from("/usr/local/bin").join(BIN_NAME)); + // Homebrew on Apple silicon. Intel's /usr/local/bin is already above. + out.push(PathBuf::from("/opt/homebrew/bin").join(BIN_NAME)); + } + + out +} + +/// Run `--version` and return the version if the binary identifies itself as +/// netscli. `None` for anything else, including a timeout or a non-zero exit. +async fn probe_version(path: &Path) -> Option { + let output = timeout(PROBE_TIMEOUT, Command::new(path).arg("--version").output()) + .await + .ok()? + .ok()?; + + if !output.status.success() { + return None; + } + + // clap prints ` `. Both halves are checked: the name is + // what rules out an unrelated program that happens to be called netscli, + // which is the case this probe exists for. + let stdout = String::from_utf8_lossy(&output.stdout); + let mut parts = stdout.split_whitespace(); + let name = parts.next()?; + if name != "netscli" { + return None; + } + parts.next().map(str::to_string) +} + +/// Find an installed `netscli`, or report that there is none. +/// +/// Returns `Ok` with empty fields rather than an error when nothing is found: +/// "the CLI is not installed" is a normal state for a desktop-only install and +/// the panel renders install guidance for it. An `Err` here would put that +/// ordinary case down the failure path. +#[tauri::command] +pub(crate) async fn detect_netscli_cli() -> Result { + for candidate in candidate_paths() { + if !candidate.is_file() { + continue; + } + if let Some(version) = probe_version(&candidate).await { + return Ok(CliDetection { + path: Some(candidate.to_string_lossy().into_owned()), + version: Some(version), + os: current_os(), + }); + } + } + + Ok(CliDetection { + path: None, + version: None, + os: current_os(), + }) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn candidates_are_named_for_the_platform() { + let expected = if cfg!(windows) { + "netscli.exe" + } else { + "netscli" + }; + let candidates = candidate_paths(); + assert!(!candidates.is_empty()); + assert!(candidates + .iter() + .all(|p| p.file_name().and_then(|n| n.to_str()) == Some(expected))); + } + + #[tokio::test] + async fn a_binary_that_is_not_netscli_is_rejected() { + // Every platform has *some* executable that answers a flag and is not + // netscli. This asserts the name check does the work, not merely that + // the process ran. + let other = if cfg!(windows) { + PathBuf::from("cmd.exe") + } else { + PathBuf::from("/bin/echo") + }; + if other.is_file() || cfg!(windows) { + assert_eq!(probe_version(&other).await, None); + } + } +} diff --git a/apps/netscli-gui/src-tauri/src/commands/mod.rs b/apps/netscli-gui/src-tauri/src/commands/mod.rs index b4aac7ef..0a7a76c6 100644 --- a/apps/netscli-gui/src-tauri/src/commands/mod.rs +++ b/apps/netscli-gui/src-tauri/src/commands/mod.rs @@ -1,4 +1,5 @@ mod files; +mod mcp; mod monitor; mod operations; @@ -7,6 +8,7 @@ pub(crate) use files::{ get_file_save_preferences, open_result_bundle, open_saved_artifact, reveal_saved_artifact, save_result_bundle, set_file_save_ask_each_time, }; +pub(crate) use mcp::detect_netscli_cli; pub(crate) use monitor::{get_default_interface, get_network_stats, list_monitorable_interfaces}; pub(crate) use operations::{ cancel_operation, capture_pcap, clear_arp_table, discover_mdns, discover_network, dns_lookup, diff --git a/apps/netscli-gui/src-tauri/src/main.rs b/apps/netscli-gui/src-tauri/src/main.rs index e5467f75..017e8185 100644 --- a/apps/netscli-gui/src-tauri/src/main.rs +++ b/apps/netscli-gui/src-tauri/src/main.rs @@ -8,8 +8,8 @@ use std::sync::Mutex; use commands::{ cancel_operation, capture_pcap, choose_file_save_default_directory, clear_arp_table, - clear_file_save_default_directory, discover_mdns, discover_network, dns_lookup, - export_text_file, get_arp_table, get_default_interface, get_file_save_preferences, + clear_file_save_default_directory, detect_netscli_cli, discover_mdns, discover_network, + dns_lookup, export_text_file, get_arp_table, get_default_interface, get_file_save_preferences, get_network_stats, inspect_host_cmd, list_interfaces, list_monitorable_interfaces, mdns_capability, open_pcap_file, open_result_bundle, open_saved_artifact, pcap_capability, ping_host, reveal_saved_artifact, reverse_dns_lookup, save_result_bundle, scan_ports, @@ -77,7 +77,8 @@ fn main() { clear_file_save_default_directory, get_network_stats, list_monitorable_interfaces, - get_default_interface + get_default_interface, + detect_netscli_cli ]) .run(tauri::generate_context!()) .expect("error while running tauri application"); diff --git a/apps/netscli-gui/src/components/shell/McpServerSection.tsx b/apps/netscli-gui/src/components/shell/McpServerSection.tsx new file mode 100644 index 00000000..89aea6bb --- /dev/null +++ b/apps/netscli-gui/src/components/shell/McpServerSection.tsx @@ -0,0 +1,153 @@ +import { Bot, Check, Copy, ExternalLink } from 'lucide-react'; +import { useEffect, useRef, useState } from 'react'; + +import { type CliDetection, detectNetscliCli } from '../../services/netscli'; + +/** Matches CommandStrip's badge duration, for the same reason: the pointer is + * already on the control, so a longer badge reads as a stuck state. */ +const COPIED_MS = 1400; + +const INSTALL_DOCS = 'https://netscli.com/docs/install/'; + +/** One command per platform, the same route the site recommends first. + * + * Duplicated from the site rather than imported -- the two are separate + * packages with no shared module -- so this is a second copy that can drift. + * Kept to ONE command per platform to bound that: the link below carries + * every other route, and the docs page is the thing that has to stay correct. + */ +const INSTALL_COMMAND: Record = { + windows: 'winget install netscli', + macos: 'brew tap fstubner/tap && brew install netscli', + linux: 'curl -fsSL https://netscli.com/install.sh | bash', +}; + +/** The block a user pastes into their MCP client. + * + * Built with JSON.stringify rather than a template literal because a Windows + * path is full of backslashes: `C:\\Users\\...` has to reach the client's + * config file escaped, and hand-writing that escaping is how it gets missed + * on the one platform most desktop users are on. + * + * The absolute path, not the bare name, for the reason the panel exists: an + * MCP client launched from the desktop session does not necessarily inherit + * the PATH a shell would, so `"command": "netscli"` fails for a client that + * a terminal would have resolved fine. */ +function clientConfig(path: string): string { + return JSON.stringify( + { mcpServers: { netscli: { command: path, args: ['serve'] } } }, + null, + 2, + ); +} + +function CopyButton({ value, label }: { value: string; label: string }) { + const [copied, setCopied] = useState(false); + const timer = useRef | null>(null); + + useEffect( + () => () => { + if (timer.current) clearTimeout(timer.current); + }, + [], + ); + + const handleClick = async () => { + if (!navigator.clipboard) return; + try { + await navigator.clipboard.writeText(value); + } catch { + return; + } + setCopied(true); + if (timer.current) clearTimeout(timer.current); + timer.current = setTimeout(() => setCopied(false), COPIED_MS); + }; + + return ( + + ); +} + +export function McpServerSection() { + const [detection, setDetection] = useState(null); + + useEffect(() => { + let cancelled = false; + void detectNetscliCli() + .then((result) => { + if (!cancelled) setDetection(result); + }) + // A detection failure and "not installed" lead to the same guidance, so + // there is nothing for the user to do differently and no error state + // worth its own copy. The os fallback keeps the install command present. + .catch(() => { + if (!cancelled) setDetection({ path: null, version: null, os: 'linux' }); + }); + return () => { + cancelled = true; + }; + }, []); + + return ( +
+ + + MCP Server + + + {detection === null ? ( +
+ Looking for the netscli command-line tool… +
+ ) : detection.path ? ( + <> +
+ Ready to connect + + Paste this into your MCP client's config to let an agent run network + scans. Found netscli {detection.version} at {detection.path} + +
+
+ 
{clientConfig(detection.path)}
+ +
+ + ) : ( + <> +
+ Command-line tool not found + + The MCP server is part of the netscli command-line tool, which installs + separately from this app. Install it and reopen Settings to get a config + block for your agent. + +
+
+ 
{INSTALL_COMMAND[detection.os]}
+ +
+ + All install options + + + + )} +
+ ); +} diff --git a/apps/netscli-gui/src/components/shell/SettingsDialog.tsx b/apps/netscli-gui/src/components/shell/SettingsDialog.tsx index 48ef7d87..90913170 100644 --- a/apps/netscli-gui/src/components/shell/SettingsDialog.tsx +++ b/apps/netscli-gui/src/components/shell/SettingsDialog.tsx @@ -13,6 +13,7 @@ import { import type { DefaultInterfaceInfo, FileSavePreferences, InterfaceInfo } from '../../types/netscli'; import type { Preferences } from '../../hooks/usePreferences'; import { useModalFocus } from '../primitives/focus'; +import { McpServerSection } from './McpServerSection'; import { NetworkActivitySection } from './NetworkActivitySection'; import { SettingsNumberInput, SettingsSwitch } from './SettingsControls'; @@ -237,6 +238,8 @@ export function SettingsDialog({ onSetTrafficPrecision={preferences.setTrafficPrecision} onToggleTrafficArrowAnimation={() => preferences.setTrafficIndicators((prev) => !prev)} /> + + diff --git a/apps/netscli-gui/src/services/netscli.ts b/apps/netscli-gui/src/services/netscli.ts index 60014a88..52e36e2b 100644 --- a/apps/netscli-gui/src/services/netscli.ts +++ b/apps/netscli-gui/src/services/netscli.ts @@ -206,3 +206,17 @@ export async function openFilesystemPath(path: string): Promise { export async function revealFilesystemPath(path: string): Promise { return invoke('reveal_saved_artifact', { path }); } + +/** Where the netscli CLI is, if it is installed at all. + * + * The desktop app does not ship the CLI, so `path` being null is an ordinary + * state rather than a failure -- see src-tauri/src/commands/mcp.rs. */ +export interface CliDetection { + path: string | null; + version: string | null; + os: 'windows' | 'macos' | 'linux'; +} + +export async function detectNetscliCli(): Promise { + return invoke('detect_netscli_cli'); +} diff --git a/apps/netscli-gui/src/styles/shell.css b/apps/netscli-gui/src/styles/shell.css index 6ea810de..0471d17a 100644 --- a/apps/netscli-gui/src/styles/shell.css +++ b/apps/netscli-gui/src/styles/shell.css @@ -6,6 +6,7 @@ @import './shell/command-status.css'; @import './shell/context-menu.css'; @import './shell/settings-controls.css'; +@import './shell/settings-mcp.css'; @import './shell/settings-number-field.css'; @import './shell/settings.css'; @import './shell/window-controls.css'; diff --git a/apps/netscli-gui/src/styles/shell/settings-mcp.css b/apps/netscli-gui/src/styles/shell/settings-mcp.css new file mode 100644 index 00000000..75af861d --- /dev/null +++ b/apps/netscli-gui/src/styles/shell/settings-mcp.css @@ -0,0 +1,77 @@ +/* The MCP section of the Settings dialog. + * + * Its own file because settings.css is at the 300-line guard and this is a + * self-contained region, the same reason settings-controls.css and + * settings-number-field.css are separate. + */ + +/* ─── MCP server section ───────────────────────────────────────────────── + * + * Two shapes share one container: a multi-line JSON config block, and a + * single-line install command. Both are things to copy, so both get the same + * treatment -- a code surface with the copy control pinned to its top-right, + * clear of the text rather than over it. + * + * `align-items: start` so the button sits level with the block's first line + * instead of centring itself against six lines of JSON. */ +.settings-mcp-config { + display: grid; + grid-template-columns: minmax(0, 1fr) auto; + align-items: start; + gap: 8px; + margin-top: 2px; + padding: 8px 10px; + border: 1px solid var(--border-subtle); + background: var(--bg-base); +} + +/* `pre`, not `code`: the config is six lines and its indentation is the + thing that makes it readable as JSON. Wrapping is allowed so a long + Windows path cannot push a horizontal scrollbar into the dialog. */ +.settings-mcp-config pre { + margin: 0; + min-width: 0; + color: var(--text-primary); + font-family: var(--font-mono); + font-size: 11px; + line-height: 1.55; + white-space: pre-wrap; + overflow-wrap: anywhere; +} + +.settings-mcp-config button { + width: 24px; + min-width: 24px; + height: 24px; +} + +.settings-mcp-link { + display: inline-flex; + align-items: center; + gap: 5px; + margin-top: 2px; + color: var(--text-muted); + font-size: 11px; + text-decoration: none; + justify-self: start; +} + +.settings-mcp-link:hover, +.settings-mcp-link:focus-visible { + color: var(--mint); +} + +/* Let this section's copy wrap. + * + * Every other settings row pairs a control with a one-line note, so + * settings.css clips `small` with `white-space: nowrap` and an ellipsis. That + * is right there and wrong here: this section explains a concept instead of + * labelling a control, and its two sentences rendered as + * "... Install it and reopen Settings t…" -- the half that tells you what to + * do was the half that got cut. */ +.settings-mcp .settings-row-copy small { + white-space: normal; + overflow: visible; + text-overflow: clip; + line-height: 1.5; +} From c36c448ee35c2f88c4ec543280e4223ddb85842c Mon Sep 17 00:00:00 2001 From: Felix Stubner Date: Thu, 17 Sep 2026 10:05:17 +0100 Subject: [PATCH 2/3] Teach the app-doc-link check about endpoint routes The check resolved a netscli.com link by looking for `.astro` or `/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. --- scripts/check-app-doc-links.mjs | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/scripts/check-app-doc-links.mjs b/scripts/check-app-doc-links.mjs index 0bbfbac0..08507570 100644 --- a/scripts/check-app-doc-links.mjs +++ b/scripts/check-app-doc-links.mjs @@ -60,9 +60,20 @@ function siteHasRoute(pathname) { const slug = pathname.replace(/^\/+/, '').replace(/\/+$/, ''); // The site root and anything under src/pages. + // + // `.ts` as well as `.astro`: src/pages also holds endpoint routes that + // return a file rather than a page -- install.sh.ts, install.ps1.ts, + // llms.txt.ts, llms-full.txt.ts. They are real built routes, and checking + // only for .astro reported the site's own canonical install one-liner + // (https://netscli.com/install.sh) as a link the site does not build. const pageCandidates = slug - ? [`${slug}.astro`, path.join(slug, 'index.astro')] - : ['index.astro']; + ? [ + `${slug}.astro`, + `${slug}.ts`, + path.join(slug, 'index.astro'), + path.join(slug, 'index.ts'), + ] + : ['index.astro', 'index.ts']; if (pageCandidates.some((candidate) => fs.existsSync(path.join(pagesRoot, candidate)))) { return true; } From d61bee1e4fbc6d5fb27a2fbcaaf2b0fd369bbb6b Mon Sep 17 00:00:00 2001 From: Felix Stubner Date: Thu, 17 Sep 2026 15:53:29 +0100 Subject: [PATCH 3/3] Fix the Tauri render harness, dead since 2026-07-14 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. --- apps/netscli-gui/e2e/tauri-render.mjs | 33 ++++- apps/netscli-gui/e2e/tauri-render/driver.mjs | 90 ++++++++++-- .../scenarios/helpers/interaction.mjs | 134 +++++++++++------- 3 files changed, 190 insertions(+), 67 deletions(-) diff --git a/apps/netscli-gui/e2e/tauri-render.mjs b/apps/netscli-gui/e2e/tauri-render.mjs index cc9a8719..ae8f7d76 100644 --- a/apps/netscli-gui/e2e/tauri-render.mjs +++ b/apps/netscli-gui/e2e/tauri-render.mjs @@ -48,11 +48,24 @@ let nativeDriverProcess; let appProcess; let probeServer; +/** Progress for the startup sequence. + * + * Between the build and the first scenario this harness used to print + * nothing, 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. These lines are cheap and make the CI log + * say where it got to. */ +function step(label) { + console.log(`[setup] ${label}`); +} + async function main() { - let webdriverPort = process.env.TAURI_DRIVER_PORT ? Number(process.env.TAURI_DRIVER_PORT) : 0; - if (!webdriverPort) { - webdriverPort = await getFreePort(); - } + // The WebDriver port is NOT chosen here. startNativeDriver picks it at the + // moment it spawns, because a port chosen now would be minutes stale by + // then -- see the note on that function. + const fixedDriverPort = process.env.TAURI_DRIVER_PORT + ? Number(process.env.TAURI_DRIVER_PORT) + : 0; const usingExternalApp = Boolean(process.env.TAURI_APP_PATH); if (usingExternalApp) { @@ -67,18 +80,27 @@ async function main() { await run(npmBin, ['run', 'build']); } + step("probe server"); const { server, port } = await startProbeServer(); probeServer = server; + step("resolve native driver"); const nativeDriverPath = await resolveNativeDriverPath(); + step("find app binary"); const application = findApplication(); process.env.NETSCLI_EXPORT_DIR = artifactsDir; // We start the app, so we choose its debug port; the driver then attaches // to it rather than launching anything. See driver.mjs for why. + step("pick debug port"); const debugPort = await getFreePort(); + step("launch app"); appProcess = await launchApplication(application, debugPort); - nativeDriverProcess = await startNativeDriver(nativeDriverPath, webdriverPort); + step("start native driver"); + const started = await startNativeDriver(nativeDriverPath, fixedDriverPort); + nativeDriverProcess = started.child; + const webdriverPort = started.port; + step("create webdriver session"); let driver; try { driver = await createDriver(webdriverPort, debugPort); @@ -96,6 +118,7 @@ async function main() { throw error; } + step("session created; setting viewport"); try { await setViewport(debugPort, DESKTOP_WINDOW); await withElement(driver, '[data-testid="app-shell"]', 20_000); diff --git a/apps/netscli-gui/e2e/tauri-render/driver.mjs b/apps/netscli-gui/e2e/tauri-render/driver.mjs index 7bb696b7..52dbf7aa 100644 --- a/apps/netscli-gui/e2e/tauri-render/driver.mjs +++ b/apps/netscli-gui/e2e/tauri-render/driver.mjs @@ -6,7 +6,7 @@ import { download as downloadEdgeDriver } from 'edgedriver'; import webdriver from 'selenium-webdriver'; import { guiRoot, repoRoot } from './paths.mjs'; -import { waitForPort } from './processes.mjs'; +import { getFreePort, stopProcess, waitForPort } from './processes.mjs'; export const { Builder, By, Key, until } = webdriver; @@ -93,20 +93,39 @@ export async function launchApplication(application, debugPort) { child.getOutput = () => output; child.profileDir = profileDir; - await Promise.race([ - waitForPort(debugPort, 60_000), - new Promise((_, reject) => { - child.once('exit', (code, signal) => { - reject(new Error(`app exited before opening its debug port (${code ?? signal})\n${output}`)); - }); - }), - ]); + // Kill the app if it never becomes usable. + // + // This used to let the rejection escape with the child still running, and + // the child is spawned with piped stdio, so those pipes kept node's event + // loop alive: the CI job printed "Timed out waiting for port" and then sat + // there for 21 more minutes until the 30-minute job cap killed it. The + // caller cannot clean this up, because `appProcess` is only assigned from + // this function's return value -- on the throw path it stays undefined and + // `stopProcess(appProcess)` is a no-op. + try { + await Promise.race([ + waitForPort(debugPort, 60_000), + new Promise((_, reject) => { + child.once('exit', (code, signal) => { + reject(new Error(`app exited before opening its debug port (${code ?? signal})\n${output}`)); + }); + }), + ]); + } catch (error) { + stopProcess(child); + throw error; + } return child; } /** Run the platform WebDriver directly; `tauri-driver` is not in the path. */ -export async function startNativeDriver(nativeDriverPath, webdriverPort) { +/** msedgedriver's own words when the port it was given is already bound. */ +function isPortCollision(message) { + return /bind\(\) returned an error|port not available/i.test(message); +} + +function spawnNativeDriver(nativeDriverPath, webdriverPort) { let output = ''; const child = spawn( nativeDriverPath, @@ -118,16 +137,61 @@ export async function startNativeDriver(nativeDriverPath, webdriverPort) { child.stderr.on('data', (chunk) => { output += chunk.toString(); }); child.getDriverOutput = () => output; - await Promise.race([ + return Promise.race([ waitForPort(webdriverPort), new Promise((_, reject) => { child.once('exit', (code, signal) => { reject(new Error(`native driver exited early with ${code ?? signal}\n${output}`)); }); }), - ]); + ]) + .then(() => child) + .catch((error) => { + stopProcess(child); + throw error; + }); +} - return child; +/** + * Start msedgedriver, choosing its port HERE rather than accepting one chosen + * earlier. + * + * The port used to be picked at the top of main(), then handed to this + * function minutes later -- after the Rust build and after the app launched. + * `getFreePort` reserves nothing: it binds an ephemeral port, reads it, and + * closes, so the number is only a fact about the instant it was taken. + * WebView2 starts a swarm of processes that take ephemeral ports, and one of + * them would take that one. msedgedriver then failed to bind and 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 directly by holding the port open and spawning the driver on it. + * + * Picking it here shrinks the window to milliseconds, and the retry closes + * what is left: the race cannot be eliminated, only made small and survivable. + * An explicit TAURI_DRIVER_PORT is honoured and never retried -- if a port was + * named, failing to get it is the answer, not a reason to use a different one. + */ +export async function startNativeDriver(nativeDriverPath, fixedPort) { + const attempts = fixedPort ? 1 : 3; + let lastError; + + for (let attempt = 1; attempt <= attempts; attempt += 1) { + const port = fixedPort || (await getFreePort()); + try { + const child = await spawnNativeDriver(nativeDriverPath, port); + return { child, port }; + } catch (error) { + lastError = error; + if (!isPortCollision(error.message)) throw error; + console.warn(`native driver could not bind port ${port}; retrying (${attempt}/${attempts})`); + } + } + + throw lastError; } export async function createDriver(webdriverPort, debugPort) { diff --git a/apps/netscli-gui/e2e/tauri-render/scenarios/helpers/interaction.mjs b/apps/netscli-gui/e2e/tauri-render/scenarios/helpers/interaction.mjs index 82abe81f..7d06752d 100644 --- a/apps/netscli-gui/e2e/tauri-render/scenarios/helpers/interaction.mjs +++ b/apps/netscli-gui/e2e/tauri-render/scenarios/helpers/interaction.mjs @@ -68,39 +68,21 @@ async function assertThemedTooltips(driver) { assert.equal(state.nativeTitles.length, 0, `Native title tooltips should not be used: ${state.nativeTitles.join(', ')}`); assert.ok(state.tooltipCount >= 6, `Expected themed tooltip hooks, got ${state.tooltipCount}`); assert.equal(state.disabledTooltipHostOpacity, '1', 'Disabled toolbar buttons should not fade their themed tooltips'); - await dispatchTooltipPointerOver(driver, '[data-testid="run-active-tab"]'); - await waitForText(driver, '[data-testid="app-tooltip"]', /Start Scan|Run|Lookup/i); - const tooltipLayer = await driver.executeScript(` - const tooltip = document.querySelector('[data-testid="app-tooltip"]'); - if (!tooltip) return null; - return { - position: getComputedStyle(tooltip).position, - zIndex: Number(getComputedStyle(tooltip).zIndex), - }; - `); - assert.ok(tooltipLayer, 'Global tooltip should render'); - assert.equal(tooltipLayer.position, 'fixed', 'Tooltips should be fixed-layer, not clipped by tab overflow'); - assert.ok(tooltipLayer.zIndex >= 1000, `Tooltip should sit above app overlays, got z-index ${tooltipLayer.zIndex}`); - await dispatchTooltipPointerOver(driver, '.detail-actions button:last-child'); - await waitForText(driver, '[data-testid="app-tooltip"]', /details pane/i); - const tooltipBounds = await driver.executeScript(` - const tooltip = document.querySelector('[data-testid="app-tooltip"]'); - if (!tooltip) return null; - const rect = tooltip.getBoundingClientRect(); - return { - left: rect.left, - right: rect.right, - top: rect.top, - bottom: rect.bottom, - width: window.innerWidth, - height: window.innerHeight, - }; - `); - assert.ok(tooltipBounds, 'Tooltip bounds should be measurable'); - assert.ok(tooltipBounds.left >= 0, `Tooltip should not be clipped on the left: ${tooltipBounds.left}`); - assert.ok(tooltipBounds.right <= tooltipBounds.width, `Tooltip should not be clipped on the right: ${tooltipBounds.right}/${tooltipBounds.width}`); - assert.ok(tooltipBounds.top >= 0, `Tooltip should not be clipped at the top: ${tooltipBounds.top}`); - assert.ok(tooltipBounds.bottom <= tooltipBounds.height, `Tooltip should not be clipped at the bottom: ${tooltipBounds.bottom}/${tooltipBounds.height}`); + const runTooltip = await hoverAndReadTooltip( + driver, + '[data-testid="run-active-tab"]', + /Start Scan|Run|Lookup/, + ); + assert.ok(!runTooltip.error, `Global tooltip should render: ${runTooltip.error}`); + assert.equal(runTooltip.position, 'fixed', 'Tooltips should be fixed-layer, not clipped by tab overflow'); + assert.ok(runTooltip.zIndex >= 1000, `Tooltip should sit above app overlays, got z-index ${runTooltip.zIndex}`); + + const bounds = await hoverAndReadTooltip(driver, '.detail-actions button:last-child', /details pane/); + assert.ok(!bounds.error, `Tooltip bounds should be measurable: ${bounds.error}`); + assert.ok(bounds.left >= 0, `Tooltip should not be clipped on the left: ${bounds.left}`); + assert.ok(bounds.right <= bounds.viewportWidth, `Tooltip should not be clipped on the right: ${bounds.right}/${bounds.viewportWidth}`); + assert.ok(bounds.top >= 0, `Tooltip should not be clipped at the top: ${bounds.top}`); + assert.ok(bounds.bottom <= bounds.viewportHeight, `Tooltip should not be clipped at the bottom: ${bounds.bottom}/${bounds.viewportHeight}`); } async function assertInteractiveCursorTreatment(driver) { @@ -122,26 +104,80 @@ async function assertInteractiveCursorTreatment(driver) { assert.equal(state.disabledCursor, 'not-allowed', 'Disabled toolbar actions should advertise disabled affordance'); } -async function dispatchTooltipPointerOver(driver, selector) { - const dispatched = await driver.executeScript( +/** + * Hover a control and read its tooltip in ONE round trip. + * + * The three-step version of this -- dispatch, `waitForText`, then a separate + * `executeScript` to measure -- raced and lost. AppTooltip hides 40ms after a + * `pointerout` (see the close timer in AppTooltip.tsx), and each WebDriver + * round trip costs far more than 40ms, so anything producing a real pointerout + * between the wait and the measurement took the tooltip away. The wait passed, + * the measurement then found nothing, and the failure read as "Global tooltip + * should render" -- which sounds like the tooltip is broken rather than like a + * test that looked too late. + * + * Everything now happens inside the page: the hover is re-asserted on every + * animation frame until the text matches, so the hide timer is continually + * cancelled no matter what triggered it, and the measurement is taken in the + * same tick as the match. A real failure still reports, and now says what the + * tooltip actually held. + */ +async function hoverAndReadTooltip(driver, selector, pattern) { + return driver.executeAsyncScript( ` - const control = document.querySelector(arguments[0]); - if (!control) return false; - const rect = control.getBoundingClientRect(); - const EventCtor = window.PointerEvent ?? MouseEvent; - control.dispatchEvent(new EventCtor('pointerover', { - bubbles: true, - cancelable: true, - composed: true, - clientX: rect.left + rect.width / 2, - clientY: rect.top + rect.height / 2, - })); - if (typeof control.focus === 'function') control.focus({ preventScroll: true }); - return true; + const selector = arguments[0]; + const source = arguments[1]; + const done = arguments[arguments.length - 1]; + const wanted = new RegExp(source, 'i'); + const control = document.querySelector(selector); + if (!control) return done({ error: 'no control matching ' + selector }); + + const hover = () => { + const rect = control.getBoundingClientRect(); + const EventCtor = window.PointerEvent ?? MouseEvent; + control.dispatchEvent(new EventCtor('pointerover', { + bubbles: true, + cancelable: true, + composed: true, + clientX: rect.left + rect.width / 2, + clientY: rect.top + rect.height / 2, + })); + if (typeof control.focus === 'function') control.focus({ preventScroll: true }); + }; + + hover(); + const deadline = Date.now() + 4000; + (function poll() { + const tooltip = document.querySelector('[data-testid="app-tooltip"]'); + const text = tooltip ? tooltip.textContent || '' : ''; + if (tooltip && wanted.test(text)) { + const style = getComputedStyle(tooltip); + const box = tooltip.getBoundingClientRect(); + return done({ + text, + position: style.position, + zIndex: Number(style.zIndex), + left: box.left, + right: box.right, + top: box.top, + bottom: box.bottom, + viewportWidth: window.innerWidth, + viewportHeight: window.innerHeight, + }); + } + if (Date.now() > deadline) { + return done({ + error: 'tooltip never matched ' + wanted + '; last text was ' + + (tooltip ? JSON.stringify(text) : '(no tooltip element)'), + }); + } + hover(); + requestAnimationFrame(poll); + })(); `, selector, + pattern.source, ); - assert.equal(dispatched, true, `Expected tooltip host ${selector} to exist`); } async function assertSuppressesNativeContextMenu(driver) {