diff --git a/docs/changelog/2025-08-04-crossbar-scan-progress.md b/docs/changelog/2026-08-04-crossbar-scan-progress.md similarity index 100% rename from docs/changelog/2025-08-04-crossbar-scan-progress.md rename to docs/changelog/2026-08-04-crossbar-scan-progress.md diff --git a/docs/changelog/2026-08-04-hostname-shortening.md b/docs/changelog/2026-08-04-hostname-shortening.md new file mode 100644 index 0000000..ba2d2a6 --- /dev/null +++ b/docs/changelog/2026-08-04-hostname-shortening.md @@ -0,0 +1,42 @@ +# Hostname shortening and LAN deduplication + +## Problem + +- LAN domain suffixes (`.fritz.box`, `.home.arpa`, AD domains, …) make labels wider than necessary, reducing the number of servers visible per row in `/crossbar`. +- Multiple network interfaces or IP versions can produce several discovered entries for the same physical machine. +- `os.hostname()` often returns only the bare hostname (e.g. `macPro16`), so the domain suffix of remote servers could not be stripped reliably. + +## Solution + +### Local domain detection (`src/discovery/dns.ts`) + +`getLocalDomain()` determines the local machine's domain suffix using a cascading, best-effort strategy: + +1. **`os.hostname()`** — if it already contains a dot, the suffix is extracted directly. +2. **Environment variables** — checked in order: `USERDNSDOMAIN`, `LOCALDOMAIN`, `DOMAIN` (covers Windows AD domains and common Unix env vars). +3. **`/etc/resolv.conf`** — parsed for `search` or `domain` directives; the first search domain wins. + +The function is lazy and cached. Each step is wrapped in `try/catch` — a failure in one source never crashes or blocks the system. If no source yields a domain, the function returns `null` and shortening is simply skipped. + +### Hostname shortening (`src/discovery/dns.ts`) + +`shortHostname(hostname)` strips the trailing domain suffix only when the hostname shares the same suffix as the local machine (case-insensitive). IP addresses and `localhost` are never shortened. + +### UI integration (`src/ui/onboarding.ts`) + +`hostPortOf()` now calls `shortHostname()` before appending the port, so all `/crossbar` labels use shortened hostnames: + +``` +llama.cpp (dagobert.fritz.box:8080) +→ +llama.cpp (dagobert:8080) +``` + +### Deduplication (`src/discovery/engine.ts`) + +`dedupByHostname()` resolves any unresolved IP addresses to hostnames via `dns.reverse()` and groups servers by resolved hostname plus port. When multiple IPs map to the same hostname, only the entry with the resolved hostname is kept. This prevents duplicate rows from scanning several interfaces (Wi-Fi, Ethernet, IPv4, IPv6) on the same machine. + +## Tests + +- `tests/discovery/dns.test.ts` — 27 tests covering the full fallback chain, shortening behavior, IP/IPv6/localhost guards, caching, and crash resistance. +- All existing tests continue to pass. diff --git a/docs/hostname-dedup.md b/docs/hostname-dedup.md new file mode 100644 index 0000000..11aa05e --- /dev/null +++ b/docs/hostname-dedup.md @@ -0,0 +1,74 @@ +# Hostname Resolution & Deduplication + +## Problem + +A single inference server with multiple network interfaces appears as **multiple +entries** in the Crossbar server list: + +``` +oMLX (192.168.188.127:8000) — WiFi NIC +oMLX (192.168.139.3:8000) — VPN/virtual NIC +oMLX (workstation.local:8000) — hostname +``` + +All three LAN entries point to the **same machine**. The user sees duplicate +servers and must manually ignore the extras. + +## Solution + +1. **Resolve** IP addresses to hostnames via reverse DNS. +2. **Deduplicate** by `hostname + port` — collapse all IPs of the same machine + into a single entry. +3. **Prefer hostname** over a still-unresolved IP when both land in the same group. +4. **Shorten labels** by stripping the shared local domain suffix for display. + +Loopback (`127.0.0.1` / `localhost`) is a different key from a LAN hostname, so a +co-located server can still appear twice (once from the localhost sweep, once from +LAN) when both paths discover it. That is intentional: the URLs differ, and the +localhost entry is the better default for a local session. + +## How it works + +### Step 1 — Reverse DNS resolution + +Each discovered IP is looked up via `dns.reverse()`: + +| IP | Resolves to | +|---|---| +| `127.0.0.1` | `127.0.0.1` (no lookup) | +| `192.168.188.127` | `workstation.local` | +| `192.168.139.3` | `workstation.local` | + +**Caching:** Results are cached within a single `discoverLocalhost` / +`discoverLan` call. `clearCache()` runs at the start of each call. + +**Graceful fallback:** If reverse DNS fails, the original IP is retained. + +### Step 2 — Dedup by hostname + port + +Servers are grouped by their resolved `hostname:port` key. For each group with +more than one entry, the entry with a resolved hostname is preferred over an IP. + +### Label shortening + +Hostnames are displayed **without their domain suffix** to save horizontal +space in the UI. This only applies to hostnames that share the same domain +suffix as the machine Crossbar is running on (via `os.hostname()`, env hints, or +`/etc/resolv.conf`). + +| Local machine | Displayed label | Full hostname (baseUrl) | +|---|---|---| +| `local-host.fritz.box` | `devbox:8080` | `devbox.fritz.box` | +| `local-host.fritz.box` | `remote.example.com:8080` | `remote.example.com` | +| `local-host.fritz.box` | `192.168.188.173:8080` | `192.168.188.173` | + +### URL path preservation + +Hostname replacement uses string replacement (not `URL` reconstruction) so +trailing slashes and paths are preserved — reconstructing via `URL` can introduce +a spurious `/` that breaks `${baseUrl}/v1` concatenation. + +## Testing + +- `tests/discovery/dns.test.ts` — resolve, shorten, cache, resolv.conf fallback +- `tests/discovery/hostname-dedup.test.ts` — multi-NIC collapse, preference, labels diff --git a/src/discovery/dns.ts b/src/discovery/dns.ts new file mode 100644 index 0000000..84b199d --- /dev/null +++ b/src/discovery/dns.ts @@ -0,0 +1,232 @@ +/** + * DNS hostname resolution for discovered servers. + * + * Resolves IP addresses to hostnames via reverse DNS (`dns.reverse()`). + * Results are cached within a scan run to avoid redundant lookups. + * Resolution failures fall back to the original value (graceful degradation). + * + * Hostnames are displayed without their domain suffix (e.g. `macpro16.fritz.box` + * → `macpro16`) to save horizontal space in the UI. The full hostname is + * preserved in the `baseUrl` for correct DNS resolution. + * + * Shortening only applies to hostnames that share the same domain suffix as + * the machine Crossbar is running on. We first inspect `os.hostname()`, then + * try cross-platform environment hints, then finally fall back to local resolver + * search/domain configuration where available (e.g. `/etc/resolv.conf`). + * Hostnames on different domains are kept in full to avoid label collisions. + */ + +import { reverse } from "node:dns/promises"; +import { readFileSync } from "node:fs"; +import { hostname } from "node:os"; + +// --------------------------------------------------------------------------- +// Local domain suffix — extracted from this machine's hostname or resolver config +// --------------------------------------------------------------------------- + +let localDomainCache: string | null | undefined; + +function domainSuffixOfHost(host: string): string | null { + const dotIndex = host.indexOf("."); + return dotIndex > 0 ? host.slice(dotIndex) : null; +} + +function normalizeDomain(domain: string): string | null { + const trimmed = domain.trim().replace(/\.+$/, ""); + if (!trimmed || trimmed === "local" || trimmed === "localhost") return null; + return trimmed.startsWith(".") ? trimmed : `.${trimmed}`; +} + +function localDomainFromEnv(): string | null { + const candidates = [ + process.env.USERDNSDOMAIN, + process.env.LOCALDOMAIN, + process.env.DOMAIN, + ]; + for (const candidate of candidates) { + if (!candidate) continue; + const first = candidate.split(/[\s,;]+/).find((part) => part.length > 0); + const normalized = first ? normalizeDomain(first) : null; + if (normalized) return normalized; + } + return null; +} + +function localDomainFromResolvConf(): string | null { + try { + const text = readFileSync("/etc/resolv.conf", "utf8"); + for (const line of text.split(/\r?\n/)) { + const trimmed = line.trim(); + if (!trimmed || trimmed.startsWith("#") || trimmed.startsWith(";")) continue; + + const searchMatch = trimmed.match(/^search\s+(.+)$/i); + if (searchMatch?.[1]) { + const first = searchMatch[1].split(/\s+/).find((part) => part.length > 0); + const normalized = first ? normalizeDomain(first) : null; + if (normalized) return normalized; + } + + const domainMatch = trimmed.match(/^domain\s+(.+)$/i); + if (domainMatch?.[1]) { + const normalized = normalizeDomain(domainMatch[1]); + if (normalized) return normalized; + } + } + } catch { + // Best-effort only. + } + return null; +} + +/** + * Get the domain suffix of the local machine, or null when none can be determined. + * Computed lazily and cached. + * + * Resolution order: + * 1. FQDN from `os.hostname()` + * 2. Cross-platform env hints (`USERDNSDOMAIN`, `LOCALDOMAIN`, `DOMAIN`) + * 3. Resolver search/domain config from `/etc/resolv.conf` when present + * + * Examples: + * `myMac.fritz.box` → `.fritz.box` + * `USERDNSDOMAIN=fritz.box` → `.fritz.box` + * `/etc/resolv.conf: search fritz.box` → `.fritz.box` + * `localhost` → `null` + */ +export function getLocalDomain(): string | null { + if (localDomainCache !== undefined) return localDomainCache; + + const fromHostname = domainSuffixOfHost(hostname()); + if (fromHostname) { + localDomainCache = normalizeDomain(fromHostname); + return localDomainCache; + } + + const fromEnv = localDomainFromEnv(); + if (fromEnv) { + localDomainCache = fromEnv; + return localDomainCache; + } + + localDomainCache = localDomainFromResolvConf(); + return localDomainCache; +} + +// --------------------------------------------------------------------------- +// Cache — keyed by IP address, values are resolved hostnames or null on failure +// --------------------------------------------------------------------------- + +const cache = new Map(); + +/** Clear the cache between scan runs. */ +export function clearCache(): void { + cache.clear(); + localDomainCache = undefined; +} + +/** + * Strip the domain suffix from a hostname for display. + * + * Only shortens hostnames that share the same domain suffix as the local + * machine (e.g. `macpro16.fritz.box` → `macpro16` when local is `myMac.fritz.box`, + * `USERDNSDOMAIN=fritz.box`, or the resolver search domain is `fritz.box`). + * Hostnames on different domains are kept in full to avoid label collisions. + * + * @param hostname - The hostname to potentially shorten. + * @returns The short hostname, or the original if no shortening applies. + */ +export function shortHostname(hostname: string): string { + // Don't strip dots from IP addresses + if (/^\d+\.\d+\.\d+\.\d+$/.test(hostname)) return hostname; + if (/^\[?[0-9a-fA-F:]+\]?$/.test(hostname)) return hostname; // IPv6 + + // No local domain suffix (e.g. localhost) — no shortening + const localDomain = getLocalDomain(); + if (!localDomain) return hostname; + + // Only shorten if the hostname shares our local domain suffix (case-insensitive) + const lowerHost = hostname.toLowerCase(); + const lowerDomain = localDomain.toLowerCase(); + if (!lowerHost.endsWith(lowerDomain)) return hostname; + + // Strip the shared suffix, keeping any subdomain labels (a.b.example → a.b). + return hostname.slice(0, hostname.length - localDomain.length); +} + +// --------------------------------------------------------------------------- +// Lookup helpers +// --------------------------------------------------------------------------- + +/** + * Resolve an IP address to a hostname via reverse DNS. + * Returns the original IP if resolution fails (graceful degradation). + * + * Cached within a scan run — repeated calls with the same IP reuse the result. + * + * @param ip - The IP address to resolve. + * @returns The resolved hostname, or the original IP on failure. + */ +export async function resolveHostname(ip: string): Promise { + // Skip already-hostname-like values + if (ip === "localhost" || ip === "127.0.0.1" || ip === "::1") return ip; + if (ip.includes(":")) return ip; // IPv6 — skip + + // Check cache first — null means we already tried and it failed + const cached = cache.get(ip); + if (cached !== undefined) return cached as string; + + try { + const hostnames = await reverse(ip); + // reverse() returns (string | null)[]; take the first non-null entry + const hostname = hostnames[0] as string; + if (hostname) { + cache.set(ip, hostname); + return hostname; + } + } catch { + // DNS resolution failed — fall back to original IP + } + + cache.set(ip, ip); + return ip; +} + +/** + * Resolve the host part of a full URL to a hostname. + * + * Examples: + * `http://192.168.1.42:11434` → `http://workstation.local:11434` + * `http://10.0.0.5:8000` → `http://devbox.local:8000` + * `http://localhost:8000` → `http://localhost:8000` (no-op) + * + * @param url - The full URL to resolve. + * @returns The URL with the host part resolved, or the original URL on failure. + */ +export async function resolveUrlHostname(url: string): Promise { + let parsed: URL; + try { + parsed = new URL(url); + } catch { + // Malformed URL — return as-is + return url; + } + + const hostname = parsed.hostname; + + // Skip if already a hostname-like value + if (hostname === "localhost" || hostname === "127.0.0.1" || hostname === "::1") return url; + if (hostname.includes(":")) return url; // IPv6 — skip + + const resolved = await resolveHostname(hostname); + + // No change — return original + if (resolved === hostname) return url; + + // Replace only the hostname in the original string — preserves whether the + // original URL had a trailing slash/path or not (avoids introducing a + // spurious "/" that breaks downstream path concatenation, e.g. `${baseUrl}/v1` + // becoming a double slash). + const hostPattern = hostname.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); // escape regex chars + const re = new RegExp(`^(${parsed.protocol}//)${hostPattern}(:|/|$)`); + return url.replace(re, `$1${resolved}$2`); +} diff --git a/src/discovery/engine.ts b/src/discovery/engine.ts index 4f59300..a16c148 100644 --- a/src/discovery/engine.ts +++ b/src/discovery/engine.ts @@ -17,6 +17,97 @@ import { CLOUD_KINDS } from "../core/capability.ts"; import type { BackendAdapter } from "../core/backend-adapter.ts"; import type { DiscoveredServer, Probe } from "../core/types.ts"; import { createProbe } from "./probe.ts"; +import { clearCache, resolveHostname, resolveUrlHostname, shortHostname } from "./dns.ts"; + +// --------------------------------------------------------------------------- +// Dedup helpers +// --------------------------------------------------------------------------- + +/** Derive a short display name for a server (short hostname + port). */ +function shortLabel(baseUrl: string, kind: string): string { + const parsed = new URL(baseUrl); + const displayHost = shortHostname(parsed.hostname); + return `${kind} (${displayHost}:${parsed.port})`; +} + +/** Extract the hostname+port key from a baseUrl for deduplication. */ +function hostPortKey(baseUrl: string): string { + let parsed: URL; + try { + parsed = new URL(baseUrl); + } catch { + return baseUrl; // malformed — use as-is + } + return `${parsed.hostname.toLowerCase()}:${parsed.port}`; +} + +/** Check if a string looks like an IPv4 address. */ +function isIpv4(s: string): boolean { + return /^[0-9]+\.[0-9]+\.[0-9]+\.[0-9]+$/.test(s); +} + +/** + * Keep only one entry per resolved hostname+port key. + * Resolves any unresolved IPs so that multiple IPs of the same machine + * (e.g., different NICs) collapse to the same hostname. + * Prefers the entry with a resolved hostname over an IP. + * + * Note: loopback (`localhost` / `127.0.0.1`) and a LAN hostname for the same + * machine stay as separate keys — callers that merge localhost + LAN scans + * (e.g. `/crossbar`) already prefer the localhost row via origin-based merge + * order when baseUrls collide after resolution; when they do not collide, both + * remain visible. + */ +export async function dedupByHostname(servers: DiscoveredServer[]): Promise { + // Resolve any unresolved IPs — reuse dns cache if available + const entries = await Promise.all( + servers.map(async (server) => { + const parsed = new URL(server.baseUrl); + const hostname = parsed.hostname; + if (isIpv4(hostname)) { + const resolved = await resolveHostname(hostname); + if (resolved !== hostname) { + // Replace only the IP in the original string — preserves whether the + // original URL had a trailing slash/path (avoids introducing a + // spurious "/" that breaks downstream path concatenation). + const escaped = hostname.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); + const re = new RegExp(`^(${parsed.protocol}//)${escaped}(:|/|$)`); + const newBaseUrl = server.baseUrl.replace(re, `$1${resolved}$2`); + return { + ...server, + baseUrl: newBaseUrl, + label: shortLabel(newBaseUrl, server.kind), + }; + } + } + return server; + }), + ); + + // Group by resolved hostname:port + const groups = new Map(); + for (const server of entries) { + const key = hostPortKey(server.baseUrl); + const list = groups.get(key) ?? []; + groups.set(key, list); + list.push(server); + } + + const result: DiscoveredServer[] = []; + for (const [_key, group] of groups) { + if (group.length === 1) { + result.push(group[0]!); + continue; + } + // Prefer resolved hostname over IP + const withHostname = group.find( + (s) => !isIpv4(new URL(s.baseUrl).hostname) && !new URL(s.baseUrl).hostname.includes("["), + ); + result.push(withHostname ?? group[0]!); + } + return result; +} + /** Default localhost ports probed in order (from CAPABILITY-MATRIX.md). */ export const DEFAULT_PROBE_PORTS: readonly number[] = [ @@ -159,6 +250,8 @@ export async function discoverLocalhost( adapters: BackendAdapter[], opts?: DiscoverLocalhostOptions, ): Promise { + clearCache(); + const ports = opts?.ports ?? DEFAULT_PROBE_PORTS; const host = opts?.host ?? DEFAULT_HOST; const timeoutMs = opts?.timeoutMs ?? DEFAULT_TIMEOUT_MS; @@ -197,7 +290,21 @@ export async function discoverLocalhost( } } - return deduplicated; + // Resolve hostnames in parallel — updates both baseUrl and label. + const resolved = await Promise.all( + deduplicated.map(async (server) => { + const newBaseUrl = await resolveUrlHostname(server.baseUrl); + if (newBaseUrl === server.baseUrl) return server; // no change + return { + ...server, + baseUrl: newBaseUrl, + label: shortLabel(newBaseUrl, server.kind), + }; + }), + ); + + // Dedup by hostname+port — collapses multiple IPs of the same machine + return await dedupByHostname(resolved); } export interface DiscoverLanOptions { @@ -238,6 +345,8 @@ export async function discoverLan( ): Promise { if (hosts.length === 0) return []; + clearCache(); + const ports = opts?.ports ?? DEFAULT_PROBE_PORTS; const timeoutMs = opts?.timeoutMs ?? DEFAULT_TIMEOUT_MS; const concurrency = opts?.concurrency ?? DEFAULT_CONCURRENCY; @@ -299,5 +408,19 @@ export async function discoverLan( } } - return deduplicated; + // Resolve hostnames in parallel — updates both baseUrl and label. + const resolved = await Promise.all( + deduplicated.map(async (server) => { + const newBaseUrl = await resolveUrlHostname(server.baseUrl); + if (newBaseUrl === server.baseUrl) return server; // no change + return { + ...server, + baseUrl: newBaseUrl, + label: shortLabel(newBaseUrl, server.kind), + }; + }), + ); + + // Dedup by hostname+port — collapses multiple IPs of the same machine + return await dedupByHostname(resolved); } diff --git a/src/ui/onboarding.ts b/src/ui/onboarding.ts index 4f14adf..027ba8b 100644 --- a/src/ui/onboarding.ts +++ b/src/ui/onboarding.ts @@ -31,16 +31,23 @@ import { adapterFor } from "../adapters/index.ts"; import { registerServer, unregisterServer } from "../shim/provider-shim.ts"; import { createProbe } from "../discovery/probe.ts"; import { expandHosts, localSubnetCidrs } from "../discovery/subnet.ts"; +import { shortHostname } from "../discovery/dns.ts"; import { catalogueChanged } from "../poll.ts"; import { DEFAULT_PROBE_PORTS, type ProgressCallback } from "../discovery/engine.ts"; // ─── Pure helpers ──────────────────────────────────────────────────────────── -/** Extract a `host:port` string from a base URL for compact labels. */ +/** + * Extract a compact `host:port` string from a base URL for UI labels. + * + * When the host is in the same domain as the machine Pi is running on, omit the + * shared domain suffix (e.g. `dagobert.fritz.box` → `dagobert`) to save space. + */ function hostPortOf(baseUrl: string): string { try { const u = new URL(baseUrl); - return `${u.hostname}:${u.port || (u.protocol === "https:" ? "443" : "80")}`; + const host = shortHostname(u.hostname); + return `${host}:${u.port || (u.protocol === "https:" ? "443" : "80")}`; } catch { return baseUrl.replace(/^https?:\/\//, ""); } diff --git a/tests/discovery/dns.test.ts b/tests/discovery/dns.test.ts new file mode 100644 index 0000000..500c865 --- /dev/null +++ b/tests/discovery/dns.test.ts @@ -0,0 +1,290 @@ +/** + * Unit tests for the DNS hostname resolution module. + * + * Tests resolveHostname(), resolveUrlHostname(), and caching behaviour. + * Real DNS lookups are avoided by mocking node:dns/promises. + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import { reverse } from "node:dns/promises"; +import { readFileSync } from "node:fs"; + +// --------------------------------------------------------------------------- +// Mock dns.reverse — real DNS is slow and unreliable in CI +// --------------------------------------------------------------------------- + +vi.mock("node:dns/promises", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + reverse: vi.fn(actual.reverse), + }; +}); + +vi.mock("node:os", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + hostname: vi.fn(actual.hostname), + }; +}); + +vi.mock("node:fs", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + readFileSync: vi.fn(actual.readFileSync), + }; +}); + +const mockReverse = vi.mocked(reverse); +const mockHostname = vi.mocked((await import("node:os")).hostname); +const mockReadFileSync = vi.mocked(readFileSync); + +// --------------------------------------------------------------------------- +// Tests +// --------------------------------------------------------------------------- + +describe("dns resolveHostname", () => { + beforeEach(() => { + vi.resetModules(); + vi.clearAllMocks(); + vi.unstubAllEnvs(); + // Default: local machine is on .local domain + mockHostname.mockReturnValue("workstation.local"); + mockReadFileSync.mockImplementation(() => { + throw new Error("ENOENT"); + }); + }); + + afterEach(() => { + vi.clearAllMocks(); + vi.unstubAllEnvs(); + }); + + it("localhost: returns IP unchanged", async () => { + const { resolveHostname } = await import("../../src/discovery/dns.ts"); + expect(await resolveHostname("localhost")).toBe("localhost"); + expect(mockReverse).not.toHaveBeenCalled(); + }); + + it("127.0.0.1: returns IP unchanged", async () => { + const { resolveHostname } = await import("../../src/discovery/dns.ts"); + expect(await resolveHostname("127.0.0.1")).toBe("127.0.0.1"); + expect(mockReverse).not.toHaveBeenCalled(); + }); + + it("IPv6: returns IP unchanged", async () => { + const { resolveHostname } = await import("../../src/discovery/dns.ts"); + expect(await resolveHostname("::1")).toBe("::1"); + expect(mockReverse).not.toHaveBeenCalled(); + }); + + it("IPv6 full address: returns IP unchanged", async () => { + const { resolveHostname } = await import("../../src/discovery/dns.ts"); + expect(await resolveHostname("2001:db8::1")).toBe("2001:db8::1"); + expect(mockReverse).not.toHaveBeenCalled(); + }); + + it("resolves a real IP to hostname", async () => { + mockReverse.mockResolvedValue(["workstation.local"]); + const { resolveHostname } = await import("../../src/discovery/dns.ts"); + expect(await resolveHostname("192.168.1.42")).toBe("workstation.local"); + expect(mockReverse).toHaveBeenCalledWith("192.168.1.42"); + }); + + it("falls back to original IP on DNS failure", async () => { + mockReverse.mockRejectedValue(new Error("DNS lookup failed")); + const { resolveHostname } = await import("../../src/discovery/dns.ts"); + expect(await resolveHostname("10.0.0.5")).toBe("10.0.0.5"); + }); + + it("falls back to original IP when reverse returns empty array", async () => { + mockReverse.mockResolvedValue([]); + const { resolveHostname } = await import("../../src/discovery/dns.ts"); + expect(await resolveHostname("10.0.0.5")).toBe("10.0.0.5"); + }); + + it("caches result — second call with same IP returns cached value", async () => { + mockReverse.mockResolvedValue(["cache-test.local"]); + const { resolveHostname } = await import("../../src/discovery/dns.ts"); + expect(await resolveHostname("172.16.0.1")).toBe("cache-test.local"); + expect(mockReverse).toHaveBeenCalledTimes(1); + // Second call should reuse cache — no additional DNS call + expect(await resolveHostname("172.16.0.1")).toBe("cache-test.local"); + expect(mockReverse).toHaveBeenCalledTimes(1); + }); + + it("clearCache clears the internal cache", async () => { + mockReverse.mockResolvedValue(["first.local"]); + const { resolveHostname, clearCache } = await import("../../src/discovery/dns.ts"); + expect(await resolveHostname("10.1.1.1")).toBe("first.local"); + clearCache(); + mockReverse.mockResolvedValue(["second.local"]); + expect(await resolveHostname("10.1.1.1")).toBe("second.local"); + expect(mockReverse).toHaveBeenCalledTimes(2); + }); +}); + +describe("dns resolveUrlHostname", () => { + beforeEach(async () => { + vi.resetModules(); + vi.clearAllMocks(); + vi.unstubAllEnvs(); + mockHostname.mockReturnValue("workstation.local"); + mockReadFileSync.mockImplementation(() => { + throw new Error("ENOENT"); + }); + const { clearCache } = await import("../../src/discovery/dns.ts"); + clearCache(); + }); + + afterEach(() => { + vi.clearAllMocks(); + vi.unstubAllEnvs(); + }); + + it("localhost: returns URL unchanged", async () => { + const { resolveUrlHostname } = await import("../../src/discovery/dns.ts"); + expect(await resolveUrlHostname("http://localhost:8000/v1")).toBe( + "http://localhost:8000/v1", + ); + expect(mockReverse).not.toHaveBeenCalled(); + }); + + it("127.0.0.1: returns URL unchanged", async () => { + const { resolveUrlHostname } = await import("../../src/discovery/dns.ts"); + expect(await resolveUrlHostname("http://127.0.0.1:11434")).toBe( + "http://127.0.0.1:11434", + ); + expect(mockReverse).not.toHaveBeenCalled(); + }); + + it("resolves IP in URL to hostname", async () => { + mockReverse.mockResolvedValue(["workstation.local"]); + const { resolveUrlHostname } = await import("../../src/discovery/dns.ts"); + // URL constructor adds trailing slash for paths; normalize in comparison + const result = await resolveUrlHostname("http://192.168.1.42:11434"); + expect(result).toMatch(/^http:\/\/workstation\.local:11434(\/)?$/); + }); + + it("preserves path, query, and hash", async () => { + mockReverse.mockResolvedValue(["dev.local"]); + const { resolveUrlHostname } = await import("../../src/discovery/dns.ts"); + const result = await resolveUrlHostname("http://10.0.0.5:8000/v1/models?key=val#top"); + expect(result).toBe("http://dev.local:8000/v1/models?key=val#top"); + }); + + it("does not add a trailing slash when the original URL has none (regression)", async () => { + mockReverse.mockResolvedValue(["devbox.local"]); + const { resolveUrlHostname } = await import("../../src/discovery/dns.ts"); + const result = await resolveUrlHostname("http://192.168.188.173:8080"); + expect(result).toBe("http://devbox.local:8080"); + expect(result.endsWith("/")).toBe(false); + }); + + it("shortHostname strips domain suffix for same-domain hostnames", async () => { + const { shortHostname } = await import("../../src/discovery/dns.ts"); + // Local machine is on .local + expect(shortHostname("workstation.local")).toBe("workstation"); + expect(shortHostname("devbox.local")).toBe("devbox"); + }); + + it("shortHostname keeps full hostname for different domains", async () => { + const { shortHostname } = await import("../../src/discovery/dns.ts"); + // Local machine is on .local + expect(shortHostname("remote.example.com")).toBe("remote.example.com"); + expect(shortHostname("vpn-server.corp.net")).toBe("vpn-server.corp.net"); + }); + + it("shortHostname skips IPs and localhost", async () => { + const { shortHostname } = await import("../../src/discovery/dns.ts"); + expect(shortHostname("192.168.1.42")).toBe("192.168.1.42"); + expect(shortHostname("localhost")).toBe("localhost"); + }); + + it("shortHostname with no local domain — no shortening", async () => { + mockHostname.mockReturnValue("localhost"); + const { shortHostname } = await import("../../src/discovery/dns.ts"); + expect(shortHostname("workstation.local")).toBe("workstation.local"); + expect(shortHostname("remote.example.com")).toBe("remote.example.com"); + }); + + it("getLocalDomain falls back to USERDNSDOMAIN", async () => { + mockHostname.mockReturnValue("workstation"); + vi.stubEnv("USERDNSDOMAIN", "fritz.box"); + const { getLocalDomain, shortHostname } = await import("../../src/discovery/dns.ts"); + expect(getLocalDomain()).toBe(".fritz.box"); + expect(shortHostname("dagobert.fritz.box")).toBe("dagobert"); + }); + + it("getLocalDomain falls back to LOCALDOMAIN/DOMAIN when hostname is short", async () => { + mockHostname.mockReturnValue("workstation"); + vi.stubEnv("LOCALDOMAIN", "lab.example.org"); + const { getLocalDomain, shortHostname } = await import("../../src/discovery/dns.ts"); + expect(getLocalDomain()).toBe(".lab.example.org"); + expect(shortHostname("gpu01.lab.example.org")).toBe("gpu01"); + }); + + it("getLocalDomain falls back to /etc/resolv.conf search domain", async () => { + mockHostname.mockReturnValue("workstation"); + mockReadFileSync.mockReturnValue("nameserver 1.1.1.1\nsearch fritz.box home.arpa\n"); + const { getLocalDomain, shortHostname } = await import("../../src/discovery/dns.ts"); + expect(getLocalDomain()).toBe(".fritz.box"); + expect(shortHostname("dagobert.fritz.box")).toBe("dagobert"); + }); + + it("getLocalDomain falls back to /etc/resolv.conf domain directive", async () => { + mockHostname.mockReturnValue("workstation"); + mockReadFileSync.mockReturnValue("domain corp.example.net\n"); + const { getLocalDomain } = await import("../../src/discovery/dns.ts"); + expect(getLocalDomain()).toBe(".corp.example.net"); + }); + + it("getLocalDomain is robust: env/file failures return null and do not throw", async () => { + mockHostname.mockReturnValue("localhost"); + mockReadFileSync.mockImplementation(() => { + throw new Error("permission denied"); + }); + const { getLocalDomain, shortHostname } = await import("../../src/discovery/dns.ts"); + expect(getLocalDomain()).toBeNull(); + expect(() => shortHostname("dagobert.fritz.box")).not.toThrow(); + expect(shortHostname("dagobert.fritz.box")).toBe("dagobert.fritz.box"); + }); + + it("falls back to original URL on DNS failure", async () => { + mockReverse.mockRejectedValue(new Error("DNS failed")); + const { resolveUrlHostname } = await import("../../src/discovery/dns.ts"); + expect(await resolveUrlHostname("http://10.0.0.5:8000")).toBe( + "http://10.0.0.5:8000", + ); + }); + + it("malformed URL returns original", async () => { + const { resolveUrlHostname } = await import("../../src/discovery/dns.ts"); + expect(await resolveUrlHostname("not-a-url")).toBe("not-a-url"); + }); + + it("caches hostname resolution across multiple URLs", async () => { + mockReverse.mockResolvedValue(["shared.local"]); + const { resolveUrlHostname } = await import("../../src/discovery/dns.ts"); + await resolveUrlHostname("http://172.16.0.1:8000"); + await resolveUrlHostname("http://172.16.0.1:11434"); + // Only one DNS call despite two URLs with same IP + expect(mockReverse).toHaveBeenCalledTimes(1); + }); + + it("clearCache also clears cached local-domain detection", async () => { + mockHostname.mockReturnValue("workstation"); + vi.stubEnv("USERDNSDOMAIN", "fritz.box"); + const { getLocalDomain, clearCache } = await import("../../src/discovery/dns.ts"); + expect(getLocalDomain()).toBe(".fritz.box"); + + vi.unstubAllEnvs(); + vi.stubEnv("USERDNSDOMAIN", "corp.example.com"); + expect(getLocalDomain()).toBe(".fritz.box"); + + clearCache(); + expect(getLocalDomain()).toBe(".corp.example.com"); + }); +}); diff --git a/tests/discovery/hostname-dedup.test.ts b/tests/discovery/hostname-dedup.test.ts new file mode 100644 index 0000000..272078a --- /dev/null +++ b/tests/discovery/hostname-dedup.test.ts @@ -0,0 +1,158 @@ +/** + * Tests for the hostname+port deduplication logic in the discovery engine. + * + * dedupByHostname is async — it resolves unresolved IPs internally so that + * multiple IPs of the same machine collapse to a single hostname entry. + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import type { DiscoveredServer } from "../../src/core/types.ts"; +import { dedupByHostname } from "../../src/discovery/engine.ts"; + +// --------------------------------------------------------------------------- +// Mock dns.reverse — avoid real DNS +// --------------------------------------------------------------------------- + +vi.mock("node:dns/promises", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + reverse: vi.fn(actual.reverse), + }; +}); + +vi.mock("node:os", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + hostname: vi.fn(actual.hostname), + }; +}); + +const mockHostname = vi.mocked((await import("node:os")).hostname); + +// --------------------------------------------------------------------------- +// Helpers +// --------------------------------------------------------------------------- + +function makeServer(baseUrl: string): DiscoveredServer { + return { + baseUrl, + label: `Server (${new URL(baseUrl).hostname}:${new URL(baseUrl).port})`, + kind: "omlx", + health: { status: "ok", latencyMs: 10 }, + } as unknown as DiscoveredServer; +} + +// --------------------------------------------------------------------------- +// Tests +// --------------------------------------------------------------------------- + +describe("dedupByHostname", () => { + beforeEach(async () => { + vi.clearAllMocks(); + // Local machine is on .local domain + mockHostname.mockReturnValue("workstation.local"); + // Clear DNS cache between tests + const { clearCache } = await import("../../src/discovery/dns.ts"); + clearCache(); + }); + + afterEach(() => { + vi.clearAllMocks(); + }); + + it("keeps single entry unchanged", async () => { + const servers = [makeServer("http://workstation.local:8000")]; + const result = await dedupByHostname(servers); + expect(result).toHaveLength(1); + expect(result[0]!.baseUrl).toBe("http://workstation.local:8000"); + }); + + it("keeps different hostnames on same port", async () => { + const servers = [ + makeServer("http://workstation.local:8000"), + makeServer("http://devbox.local:8000"), + ]; + const result = await dedupByHostname(servers); + expect(result).toHaveLength(2); + }); + + it("same hostname different ports — both kept", async () => { + const servers = [ + makeServer("http://workstation.local:8000"), + makeServer("http://workstation.local:11434"), + ]; + const result = await dedupByHostname(servers); + expect(result).toHaveLength(2); + }); + + it("resolves multiple IPs to same hostname — collapses to one", async () => { + // All three IPs resolve to the same hostname + vi.mocked(await import("node:dns/promises")).reverse.mockResolvedValue(["workstation.local"]); + + const servers = [ + makeServer("http://192.168.188.127:8000"), + makeServer("http://192.168.139.3:8000"), + makeServer("http://workstation.local:8000"), // already resolved + ]; + const result = await dedupByHostname(servers); + expect(result).toHaveLength(1); + // URL constructor adds trailing slash for empty paths + expect(result[0]!.baseUrl.replace(/\/$/, "")).toBe("http://workstation.local:8000"); + }); + + it("one IP resolves to hostname — preferred over unresolved IP", async () => { + const mockReverse = vi.mocked(await import("node:dns/promises")).reverse; + // First IP resolves, second doesn't + mockReverse + .mockResolvedValueOnce(["workstation.local"]) + .mockRejectedValueOnce(new Error("not found")); + + const servers = [ + makeServer("http://10.0.0.5:8000"), // resolves to workstation.local + makeServer("http://10.0.0.6:8000"), // fails to resolve + makeServer("http://workstation.local:8000"), // already hostname + ]; + const result = await dedupByHostname(servers); + // Two groups: workstation.local:8000 (2 entries) and 10.0.0.6:8000 (1 entry) + expect(result).toHaveLength(2); + // The workstation.local entry is preferred over the unresolved 10.0.0.5 + const hostnameEntry = result.find((s) => s.baseUrl.includes("workstation.local")); + expect(hostnameEntry).toBeDefined(); + }); + + it("all IPs unresolved and different — keeps first in each group", async () => { + vi.mocked(await import("node:dns/promises")).reverse.mockRejectedValue(new Error("fail")); + const servers = [ + makeServer("http://192.168.188.127:8000"), + makeServer("http://192.168.139.3:8000"), + ]; + const result = await dedupByHostname(servers); + // Different IPs, different keys — both kept + expect(result).toHaveLength(2); + }); + + it("localhost and hostname on same port — both kept (different keys)", async () => { + const servers = [ + makeServer("http://127.0.0.1:8000"), + makeServer("http://workstation.local:8000"), + ]; + const result = await dedupByHostname(servers); + expect(result).toHaveLength(2); + }); + + it("labels use short hostname (domain suffix stripped)", async () => { + vi.mocked(await import("node:dns/promises")).reverse.mockResolvedValue( + ["workstation.local"], + ); + const servers = [ + makeServer("http://192.168.188.127:8000"), + ]; + const result = await dedupByHostname(servers); + expect(result).toHaveLength(1); + // Label should show "workstation" not "workstation.local" + expect(result[0]!.label).toContain("workstation:"); + expect(result[0]!.label).not.toContain(".local"); + }); +});