From 93ba77e38ad7bc4a85b31dc75b53eaedbef67746 Mon Sep 17 00:00:00 2001 From: Felix Stubner Date: Sat, 3 Oct 2026 20:27:33 +0100 Subject: [PATCH 1/2] Make defaults match across the CLI, TUI, desktop app and MCP server - Desktop scan and inspect default to the core's 22,80,443 instead of a five-port list of their own; sweep reuses the same constant. - MCP: one ops_config() helper. Without a client timeout each phase keeps the core default (ping 1000, scan 500, DNS 1500) instead of the tool's single advertised default standing in for all three. - MCP schemas stop advertising defaults the code does not apply (dns type A, subnet 192.168.1.0/24, start_pcap_capture outputFile). - Ping count capped at MAX_PING_COUNT in the TUI (was unbounded), the Tauri command (was 64) and the desktop form (was 50). The MCP mDNS clamp uses MAX_MDNS_TIMEOUT_MS instead of a literal. - Desktop inspect passes the concurrency preference through. - Desktop pcap duration defaults to 10 s like the core. Left alone on purpose: MCP ping count defaults to 1 (single summary for a model, documented in schemas.rs), the desktop keeps its 1,000-packet cap (its results table is not virtualised), and the desktop resolves hostnames on discover and sweep (a UI choice; the CLI keeps it opt-in for speed). --- CHANGELOG.md | 20 +++++ apps/netscli-cli/src/tui/events/host.rs | 2 +- .../src-tauri/src/commands/operations/scan.rs | 7 +- apps/netscli-gui/src/services/netscli.ts | 3 +- .../src/tools/presentation.test.ts | 2 +- apps/netscli-gui/src/tools/registry.ts | 12 +-- .../src/workspace/toolExecution.test.ts | 2 +- .../src/workspace/toolExecution.ts | 2 +- crates/netscli-mcp/src/server/operations.rs | 88 +++++++++++-------- crates/netscli-mcp/src/server/tools.rs | 13 ++- 10 files changed, 95 insertions(+), 56 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 880b3912..9c8f8340 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -88,6 +88,26 @@ its heading and collects entries; the date and the link go on with the tag. port. Replies are also sent as compact JSON instead of indented, which took a sweep of the local network from 15,399 bytes to 8,746. +- **Defaults now match across the CLI, TUI, desktop app and MCP server.** + - The desktop app scanned five ports by default (22, 80, 443, 8080 and + 8443) where everything else scans 22, 80 and 443. It now uses the same + three, for port scans and inspect alike. + - The MCP server applied the timeout it advertises to every step, so an + MCP inspect or sweep pinged with 500 ms where the others use 1,000, and + gave name lookups 500 or 1,000 ms where the others give 1,500. Without a + `timeout` each step now keeps its usual default. + - The MCP tool list advertised defaults the server did not use: an `A` + record for DNS lookups (it looks up every type), `192.168.1.0/24` for + discover and sweep (it uses your own network) and `capture.pcap` for + background captures (it names the file after the job). They now say + what happens. + - Ping count tops out at 256 everywhere. The desktop app stopped at 50 or + 64, and the TUI had no limit at all. + - Desktop inspect ignored the concurrency setting in Preferences. It now + uses it, like scan, discover and sweep. + - A desktop packet capture lasts 10 seconds by default, as on the CLI and + MCP server, instead of 5. + - **Discovery reads the Windows device table directly** instead of running `arp -a` and parsing its text. On an idle machine that makes no measurable difference (`arp -a` took 65 ms), but it removes a program start from every diff --git a/apps/netscli-cli/src/tui/events/host.rs b/apps/netscli-cli/src/tui/events/host.rs index c1ed9744..db8be3e1 100644 --- a/apps/netscli-cli/src/tui/events/host.rs +++ b/apps/netscli-cli/src/tui/events/host.rs @@ -14,7 +14,7 @@ pub(super) async fn handle_ping( .get(2) .and_then(|s| s.parse::().ok()) .unwrap_or(4) - .max(1); + .clamp(1, netscli_core::MAX_PING_COUNT); let ip = match ops.resolve_host_ip(host).await { Ok(ip) => ip, diff --git a/apps/netscli-gui/src-tauri/src/commands/operations/scan.rs b/apps/netscli-gui/src-tauri/src/commands/operations/scan.rs index ff7e296e..cc1cae80 100644 --- a/apps/netscli-gui/src-tauri/src/commands/operations/scan.rs +++ b/apps/netscli-gui/src-tauri/src/commands/operations/scan.rs @@ -1,6 +1,6 @@ use std::sync::Arc; -use netscli_core::{parse_ports_checked, Ops}; +use netscli_core::{parse_ports_checked, Ops, MAX_PING_COUNT}; use super::{ emit_operation_progress, ops_with_concurrency, run_json_operation, JsonResult, @@ -17,7 +17,7 @@ pub(crate) async fn ping_host( ) -> JsonResult { run_json_operation(op_id, manager, None, move || async move { let ops = Ops::default(); - let count = count.unwrap_or(4).clamp(1, 64); + let count = count.unwrap_or(4).clamp(1, MAX_PING_COUNT); let res = ops .ping_host_summary(host.trim(), count) .await @@ -184,10 +184,11 @@ pub(crate) async fn inspect_host_cmd( op_id: Option, host: String, ports: Option, + max_concurrent: Option, manager: tauri::State<'_, OperationManager>, ) -> JsonResult { run_json_operation(op_id, manager, None, move || async move { - let ops = Ops::default(); + let ops = ops_with_concurrency(max_concurrent); let ports = parse_ports_checked(ports.as_deref()).map_err(|e| e.to_string())?; let res = ops .inspect_host(host, ports) diff --git a/apps/netscli-gui/src/services/netscli.ts b/apps/netscli-gui/src/services/netscli.ts index c0fee50a..991cf342 100644 --- a/apps/netscli-gui/src/services/netscli.ts +++ b/apps/netscli-gui/src/services/netscli.ts @@ -78,8 +78,9 @@ export async function inspectHost( host: string, ports?: string, op_id?: string, + max_concurrent?: number, ): Promise { - return invoke('inspect_host_cmd', { opId: op_id, host, ports }); + return invoke('inspect_host_cmd', { opId: op_id, host, ports, maxConcurrent: max_concurrent }); } export async function sweepNetwork( diff --git a/apps/netscli-gui/src/tools/presentation.test.ts b/apps/netscli-gui/src/tools/presentation.test.ts index 4043e398..b034f7c9 100644 --- a/apps/netscli-gui/src/tools/presentation.test.ts +++ b/apps/netscli-gui/src/tools/presentation.test.ts @@ -68,7 +68,7 @@ describe('buildCommand', () => { it('builds command previews for every tool', () => { const scan = createTab('scan'); scan.form.host = '1.1.1.1'; - expect(buildCommand(scan)).toBe('netscli scan 1.1.1.1 -p 22,80,443,8080,8443 --json'); + expect(buildCommand(scan)).toBe('netscli scan 1.1.1.1 -p 22,80,443 --json'); const discover = createTab('discover'); discover.form.subnet = '192.168.1.0/24'; diff --git a/apps/netscli-gui/src/tools/registry.ts b/apps/netscli-gui/src/tools/registry.ts index 706aa4f8..b1799c6e 100644 --- a/apps/netscli-gui/src/tools/registry.ts +++ b/apps/netscli-gui/src/tools/registry.ts @@ -14,7 +14,9 @@ import { } from './operationIcons'; import type { DetailTab, ToolCapabilityMap, ToolConfig, ToolKind, WorkspaceTab } from './types'; -export const DEFAULT_PORTS = '22,80,443,8080,8443'; +// The core's default list (netscli_core::DEFAULT_PORTS), so a scan started +// here checks the same ports as one from the CLI, TUI or MCP server. +export const DEFAULT_PORTS = '22,80,443'; export const TOOL_KINDS: ToolKind[] = [ 'scan', @@ -71,7 +73,7 @@ export const TOOL_CONFIG: Record = { action: 'Ping', fields: [ { key: 'host', label: 'Host', placeholder: '127.0.0.1', required: true }, - { key: 'count', label: 'Count', type: 'number', compact: true, placeholder: '4', min: 1, max: 50, step: 1 }, + { key: 'count', label: 'Count', type: 'number', compact: true, placeholder: '4', min: 1, max: 256, step: 1 }, ], }, trace: { @@ -179,7 +181,7 @@ export const TOOL_CONFIG: Record = { fields: [ { key: 'mode', label: 'Mode', type: 'select', compact: true, options: ['Capture', 'Open File'] }, { key: 'interface', label: 'Interface', type: 'select', placeholder: 'Select interface', required: true }, - { key: 'duration', label: 'Seconds', type: 'number', compact: true, placeholder: '5', min: 1, max: 3600, step: 1 }, + { key: 'duration', label: 'Seconds', type: 'number', compact: true, placeholder: '10', min: 1, max: 3600, step: 1 }, { key: 'filter', label: 'Filter', placeholder: 'tcp port 443' }, { key: 'max_packets', label: 'Packets', type: 'number', compact: true, placeholder: '1000', min: 1, max: 100000, step: 1 }, ], @@ -194,11 +196,11 @@ export const DEFAULT_FORM: Record> = { dns: { host: 'netscli.com', record: 'ALL' }, reverse: { ip: '127.0.0.1' }, inspect: { host: '127.0.0.1', ports: DEFAULT_PORTS }, - sweep: { subnet: '', ports: '22,80,443' }, + sweep: { subnet: '', ports: DEFAULT_PORTS }, mdns: { timeout_ms: '3000', service_types: '' }, interfaces: {}, arp: {}, - pcap: { mode: 'Capture', interface: '', duration: '5', filter: '', max_packets: '1000' }, + pcap: { mode: 'Capture', interface: '', duration: '10', filter: '', max_packets: '1000' }, }; export const DEFAULT_SORT: Record = { diff --git a/apps/netscli-gui/src/workspace/toolExecution.test.ts b/apps/netscli-gui/src/workspace/toolExecution.test.ts index e579ffb7..4725f4e8 100644 --- a/apps/netscli-gui/src/workspace/toolExecution.test.ts +++ b/apps/netscli-gui/src/workspace/toolExecution.test.ts @@ -135,7 +135,7 @@ describe('executeTool port scan protocol', () => { const netscli = await import('../services/netscli'); const tab = createTab('scan'); await executeTool(tab, 'op-1', 256); - expect(netscli.scanPorts).toHaveBeenLastCalledWith('127.0.0.1', '22,80,443,8080,8443', 'op-1', 256, false); + expect(netscli.scanPorts).toHaveBeenLastCalledWith('127.0.0.1', '22,80,443', 'op-1', 256, false); }); it('scans UDP when the switch says so, with the ports in the field', async () => { diff --git a/apps/netscli-gui/src/workspace/toolExecution.ts b/apps/netscli-gui/src/workspace/toolExecution.ts index 71180432..62ed9510 100644 --- a/apps/netscli-gui/src/workspace/toolExecution.ts +++ b/apps/netscli-gui/src/workspace/toolExecution.ts @@ -86,7 +86,7 @@ export async function executeTool( case 'inspect': return { kind: 'inspect', - data: await netscli.inspectHost(tab.form.host.trim(), emptyToUndefined(tab.form.ports), opId), + data: await netscli.inspectHost(tab.form.host.trim(), emptyToUndefined(tab.form.ports), opId, maxConcurrentProbes), }; case 'sweep': return { diff --git a/crates/netscli-mcp/src/server/operations.rs b/crates/netscli-mcp/src/server/operations.rs index 9ed83472..c0142605 100644 --- a/crates/netscli-mcp/src/server/operations.rs +++ b/crates/netscli-mcp/src/server/operations.rs @@ -47,16 +47,32 @@ fn resolve_and_check( Ok(subnet) } -pub(super) async fn op_discover(p: DiscoverParams) -> Result, RpcError> { - let subnet = resolved_subnet(p.subnet)?; - let concurrency = clamp_concurrency(p.max_concurrent, netscli_core::DEFAULT_CONCURRENCY); - let timeout_ms = clamp_timeout_ms(p.timeout, netscli_core::DEFAULT_PING_TIMEOUT_MS); - let cfg = netscli_core::OpsConfig { +/// The settings for one operation: the client's concurrency, and its +/// `timeout` applied to every phase if it gave one. +/// +/// Without a `timeout` each phase keeps the core default (ping 1,000 ms, +/// scan 500 ms, DNS 1,500 ms), the same as the CLI, TUI and desktop app. +/// Before, the tool's advertised default stood in for all of them, so an MCP +/// inspect or sweep pinged with 500 ms where the others used 1,000, and a +/// discover gave reverse DNS 1,000 ms where the others gave 1,500. +fn ops_config(concurrency: usize, timeout: Option) -> netscli_core::OpsConfig { + let mut cfg = netscli_core::OpsConfig { concurrency, - ping_timeout_ms: timeout_ms, - dns_timeout_ms: timeout_ms, ..Default::default() }; + if timeout.is_some() { + let ms = clamp_timeout_ms(timeout, 0); + cfg.ping_timeout_ms = ms; + cfg.scan_timeout_ms = ms; + cfg.dns_timeout_ms = ms; + } + cfg +} + +pub(super) async fn op_discover(p: DiscoverParams) -> Result, RpcError> { + let subnet = resolved_subnet(p.subnet)?; + let concurrency = clamp_concurrency(p.max_concurrent, netscli_core::DEFAULT_CONCURRENCY); + let cfg = ops_config(concurrency, p.timeout); let ops = netscli_core::Ops::new(cfg); let (_subnet, hosts) = ops .discover_ipv4(Some(subnet), p.resolve_hostnames.unwrap_or(false)) @@ -70,12 +86,7 @@ pub(super) async fn op_scan_ports( ) -> Result, RpcError> { let ports = normalize_ports(p.ports)?; let concurrency = clamp_concurrency(p.max_concurrent, netscli_core::DEFAULT_CONCURRENCY); - let timeout_ms = clamp_timeout_ms(p.timeout, netscli_core::DEFAULT_SCAN_TIMEOUT_MS); - let cfg = netscli_core::OpsConfig { - concurrency, - scan_timeout_ms: timeout_ms, - ..Default::default() - }; + let cfg = ops_config(concurrency, p.timeout); // Scan the address that was checked, not the name that produced it -- // see `ensure_host_allowed` for why the two can differ. let ip = ensure_host_allowed(&p.host, netscli_core::DEFAULT_DNS_TIMEOUT_MS).await?; @@ -111,13 +122,7 @@ pub(super) async fn op_inspect_host( ) -> Result { let ports = normalize_ports(p.ports)?; let concurrency = clamp_concurrency(p.max_concurrent, netscli_core::DEFAULT_CONCURRENCY); - let timeout_ms = clamp_timeout_ms(p.timeout, netscli_core::DEFAULT_SCAN_TIMEOUT_MS); - let cfg = netscli_core::OpsConfig { - concurrency, - scan_timeout_ms: timeout_ms, - ping_timeout_ms: timeout_ms, - dns_timeout_ms: timeout_ms, - }; + let cfg = ops_config(concurrency, p.timeout); let ops = netscli_core::Ops::new(cfg); let ip = ensure_host_allowed(&p.host, netscli_core::DEFAULT_DNS_TIMEOUT_MS).await?; let mut result = ops @@ -137,13 +142,7 @@ pub(super) async fn op_sweep(p: SweepParams) -> Result Result Result, RpcError> { // Clamp timeout: no point waiting more than 30s for an interactive-like // tool call, and 0/None means use the 3s default. - let timeout_ms = p.timeout_ms.unwrap_or(3000).clamp(100, 30_000); + let timeout_ms = p + .timeout_ms + .unwrap_or(3000) + .clamp(100, netscli_core::MAX_MDNS_TIMEOUT_MS); let service_types = p.service_types.unwrap_or_default(); let ops = netscli_core::Ops::default(); ops.discover_mdns(&service_types, std::time::Duration::from_millis(timeout_ms)) @@ -241,6 +238,25 @@ mod tests { assert!(!worth_reporting(PortStatus::Filtered)); } + #[test] + fn without_a_timeout_each_phase_keeps_the_core_default() { + let cfg = ops_config(64, None); + assert_eq!(cfg.concurrency, 64); + assert_eq!(cfg.ping_timeout_ms, netscli_core::DEFAULT_PING_TIMEOUT_MS); + assert_eq!(cfg.scan_timeout_ms, netscli_core::DEFAULT_SCAN_TIMEOUT_MS); + assert_eq!(cfg.dns_timeout_ms, netscli_core::DEFAULT_DNS_TIMEOUT_MS); + } + + #[test] + fn a_timeout_from_the_client_applies_to_every_phase_within_bounds() { + let cfg = ops_config(64, Some(2_000)); + assert_eq!( + (cfg.ping_timeout_ms, cfg.scan_timeout_ms, cfg.dns_timeout_ms), + (2_000, 2_000, 2_000) + ); + assert_eq!(ops_config(64, Some(1)).scan_timeout_ms, 10); + } + #[test] fn a_publicly_addressed_interface_is_refused_rather_than_scanned() { // The case this whole change exists for. Before, the default was diff --git a/crates/netscli-mcp/src/server/tools.rs b/crates/netscli-mcp/src/server/tools.rs index e2e1494b..8a929020 100644 --- a/crates/netscli-mcp/src/server/tools.rs +++ b/crates/netscli-mcp/src/server/tools.rs @@ -10,11 +10,10 @@ pub fn tools_list() -> serde_json::Value { "properties": { "subnet": { "type": "string", - "default": "192.168.1.0/24", "description": "IPv4 CIDR, at most a /16. Defaults to the local subnet." }, "resolveHostnames": { "type": "boolean", "default": false }, - "timeout": { "type": "number", "default": 1000, "minimum": 10, "maximum": 600000 }, + "timeout": { "type": "number", "minimum": 10, "maximum": 600000, "description": "Milliseconds, applied to every step. Omit for the defaults: ping 1000, scan 500, DNS 1500." }, "maxConcurrent": { "type": "number", "default": 256, "minimum": 1, "maximum": 1024 } } }, @@ -76,7 +75,7 @@ pub fn tools_list() -> serde_json::Value { "host": { "type": "string" }, "type": { "type": "string", - "default": "A", + "description": "Record type. Omit, or pass ALL, for every type.", "enum": ["A", "AAAA", "CNAME", "MX", "NS", "TXT", "SRV", "PTR", "SOA", "CAA", "ALL", "ANY"] } }, @@ -113,7 +112,7 @@ pub fn tools_list() -> serde_json::Value { "items": { "type": "number", "minimum": 1, "maximum": 65535 }, "maxItems": 4096 }, - "timeout": { "type": "number", "default": 500, "minimum": 10, "maximum": 600000 }, + "timeout": { "type": "number", "minimum": 10, "maximum": 600000, "description": "Milliseconds, applied to every step. Omit for the defaults: ping 1000, scan 500, DNS 1500." }, "include_closed": { "type": "boolean", "default": false, "description": "Also list closed and filtered ports in ports, one entry each." }, "maxConcurrent": { "type": "number", "default": 256, "minimum": 1, "maximum": 1024 } }, @@ -131,10 +130,10 @@ pub fn tools_list() -> serde_json::Value { "inputSchema": { "type": "object", "properties": { - "subnet": { "type": "string", "default": "192.168.1.0/24" }, + "subnet": { "type": "string", "description": "IPv4 CIDR, at most a /16. Defaults to the local subnet." }, "ports": { "type": "array", "items": { "type": "number" } }, "resolveHostnames": { "type": "boolean", "default": false }, - "timeout": { "type": "number", "default": 500 }, + "timeout": { "type": "number", "minimum": 10, "maximum": 600000, "description": "Milliseconds, applied to every step. Omit for the defaults: ping 1000, scan 500, DNS 1500." }, "maxConcurrent": { "type": "number", "default": 256 } } }, @@ -191,7 +190,7 @@ pub fn tools_list() -> serde_json::Value { "interface": { "type": "string" }, "filter": { "type": "string" }, "duration": { "type": "number", "default": 10 }, - "outputFile": { "type": "string", "default": "capture.pcap" }, + "outputFile": { "type": "string", "description": "Where to write the capture. Omit for a file named after the job." }, "maxPackets": { "type": "number" } }, "required": ["interface"] From 55f86c57200293a93cc570cdeda6e3d52b53c1ac Mon Sep 17 00:00:00 2001 From: Felix Stubner Date: Sat, 3 Oct 2026 20:28:16 +0100 Subject: [PATCH 2/2] CHANGELOG: no inline colon in the defaults entry --- CHANGELOG.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9c8f8340..2cb9f8c5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -96,11 +96,11 @@ its heading and collects entries; the date and the link go on with the tag. MCP inspect or sweep pinged with 500 ms where the others use 1,000, and gave name lookups 500 or 1,000 ms where the others give 1,500. Without a `timeout` each step now keeps its usual default. - - The MCP tool list advertised defaults the server did not use: an `A` - record for DNS lookups (it looks up every type), `192.168.1.0/24` for - discover and sweep (it uses your own network) and `capture.pcap` for - background captures (it names the file after the job). They now say - what happens. + - The MCP tool list advertised defaults the server did not use. DNS + lookups claimed an `A` record and look up every type, discover and + sweep claimed `192.168.1.0/24` and use your own network, and background + captures claimed `capture.pcap` and name the file after the job. The + tool list now says what happens. - Ping count tops out at 256 everywhere. The desktop app stopped at 50 or 64, and the TUI had no limit at all. - Desktop inspect ignored the concurrency setting in Preferences. It now