From c43bfcbbecffa2566ac2dc5fe0ea5b58fdac2a1a Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 22 Mar 2026 16:24:59 -0700 Subject: [PATCH] refactor: split best-effort network display discovery --- src/cli/daemon-cli/status.gather.ts | 55 +++++------------ src/commands/gateway-status/helpers.ts | 12 +--- src/commands/onboard-helpers.test.ts | 11 +++- src/commands/onboard-helpers.ts | 27 +++------ src/gateway/net.test.ts | 4 +- src/gateway/net.ts | 4 +- src/infra/infra-runtime.test.ts | 8 --- src/infra/network-discovery-display.test.ts | 67 +++++++++++++++++++++ src/infra/network-discovery-display.ts | 66 ++++++++++++++++++++ src/infra/network-interfaces.ts | 8 ++- src/infra/system-presence.ts | 9 +-- src/infra/tailnet.test.ts | 8 +++ src/infra/tailnet.ts | 4 +- 13 files changed, 187 insertions(+), 96 deletions(-) create mode 100644 src/infra/network-discovery-display.test.ts create mode 100644 src/infra/network-discovery-display.ts diff --git a/src/cli/daemon-cli/status.gather.ts b/src/cli/daemon-cli/status.gather.ts index 5e66b69466f..3ef45b9686b 100644 --- a/src/cli/daemon-cli/status.gather.ts +++ b/src/cli/daemon-cli/status.gather.ts @@ -17,8 +17,11 @@ import { auditGatewayServiceConfig } from "../../daemon/service-audit.js"; import type { GatewayServiceRuntime } from "../../daemon/service-runtime.js"; import { resolveGatewayService } from "../../daemon/service.js"; import { isGatewaySecretRefUnavailableError, trimToUndefined } from "../../gateway/credentials.js"; -import { resolveGatewayBindHost } from "../../gateway/net.js"; import { resolveGatewayProbeAuthWithSecretInputs } from "../../gateway/probe-auth.js"; +import { + inspectBestEffortPrimaryTailnetIPv4, + resolveBestEffortGatewayBindHostForDisplay, +} from "../../infra/network-discovery-display.js"; import { parseStrictPositiveInteger } from "../../infra/parse-finite-number.js"; import { formatPortDiagnostics, @@ -26,7 +29,6 @@ import { type PortListener, type PortUsageStatus, } from "../../infra/ports.js"; -import { pickPrimaryTailnetIPv4 } from "../../infra/tailnet.js"; import { loadGatewayTlsRuntime } from "../../infra/tls/gateway.js"; import { probeGatewayStatus } from "./probe.js"; import { inspectGatewayRestart } from "./restart-health.js"; @@ -74,26 +76,6 @@ type ResolvedGatewayStatus = { probeUrlOverride: string | null; }; -function summarizeDisplayNetworkError(error: unknown): string { - if (error instanceof Error) { - const message = error.message.trim(); - if (message) { - return message; - } - } - return "network interface discovery failed"; -} - -function fallbackBindHostForStatus(bindMode: GatewayBindMode, customBindHost?: string): string { - if (bindMode === "lan") { - return "0.0.0.0"; - } - if (bindMode === "custom") { - return customBindHost?.trim() || "0.0.0.0"; - } - return "127.0.0.1"; -} - function appendProbeNote( existing: string | undefined, extra: string | undefined, @@ -104,7 +86,6 @@ function appendProbeNote( } return [...new Set(values)].join(" "); } - export type DaemonStatus = { service: { label: string; @@ -232,23 +213,14 @@ async function resolveGatewayStatusSummary(params: { : "env/config"; const bindMode: GatewayBindMode = params.daemonCfg.gateway?.bind ?? "loopback"; const customBindHost = params.daemonCfg.gateway?.customBindHost; - let bindHost: string; - let networkWarning: string | undefined; - try { - bindHost = await resolveGatewayBindHost(bindMode, customBindHost); - } catch (error) { - bindHost = fallbackBindHostForStatus(bindMode, customBindHost); - networkWarning = `Status is using fallback network details because interface discovery failed: ${summarizeDisplayNetworkError(error)}.`; - } - let tailnetIPv4: string | undefined; - try { - tailnetIPv4 = pickPrimaryTailnetIPv4(); - } catch (error) { - networkWarning = appendProbeNote( - networkWarning, - `Status could not inspect tailnet addresses: ${summarizeDisplayNetworkError(error)}.`, - ); - } + const { bindHost, warning: bindHostWarning } = await resolveBestEffortGatewayBindHostForDisplay({ + bindMode, + customBindHost, + warningPrefix: "Status is using fallback network details because interface discovery failed", + }); + const { tailnetIPv4, warning: tailnetWarning } = inspectBestEffortPrimaryTailnetIPv4({ + warningPrefix: "Status could not inspect tailnet addresses", + }); const probeHost = pickProbeHostForBind(bindMode, tailnetIPv4, customBindHost); const probeUrlOverride = trimToUndefined(params.rpcUrlOverride) ?? null; const scheme = params.daemonCfg.gateway?.tls?.enabled === true ? "wss" : "ws"; @@ -259,7 +231,8 @@ async function resolveGatewayStatusSummary(params: { : !probeUrlOverride && bindMode === "loopback" ? "Loopback-only gateway; only local clients can connect." : undefined; - probeNote = appendProbeNote(probeNote, networkWarning); + probeNote = appendProbeNote(probeNote, bindHostWarning); + probeNote = appendProbeNote(probeNote, tailnetWarning); return { gateway: { diff --git a/src/commands/gateway-status/helpers.ts b/src/commands/gateway-status/helpers.ts index 75869c8167e..0a8054729ab 100644 --- a/src/commands/gateway-status/helpers.ts +++ b/src/commands/gateway-status/helpers.ts @@ -5,7 +5,7 @@ import { hasConfiguredSecretInput } from "../../config/types.secrets.js"; import { readGatewayPasswordEnv, readGatewayTokenEnv } from "../../gateway/credentials.js"; import type { GatewayProbeResult } from "../../gateway/probe.js"; import { resolveConfiguredSecretInputString } from "../../gateway/resolve-configured-secret-input-string.js"; -import { pickPrimaryTailnetIPv4 } from "../../infra/tailnet.js"; +import { inspectBestEffortPrimaryTailnetIPv4 } from "../../infra/network-discovery-display.js"; import { colorize, theme } from "../../terminal/theme.js"; import { pickGatewaySelfPresence } from "../gateway-presence.js"; @@ -81,14 +81,6 @@ function normalizeWsUrl(value: string): string | null { return trimmed; } -function pickPrimaryTailnetIPv4ForStatus(): string | undefined { - try { - return pickPrimaryTailnetIPv4(); - } catch { - return undefined; - } -} - export function resolveTargets(cfg: OpenClawConfig, explicitUrl?: string): GatewayStatusTarget[] { const targets: GatewayStatusTarget[] = []; const add = (t: GatewayStatusTarget) => { @@ -318,7 +310,7 @@ export function extractConfigSummary(snapshotUnknown: unknown): GatewayConfigSum } export function buildNetworkHints(cfg: OpenClawConfig) { - const tailnetIPv4 = pickPrimaryTailnetIPv4ForStatus(); + const { tailnetIPv4 } = inspectBestEffortPrimaryTailnetIPv4(); const port = resolveGatewayPort(cfg); return { localLoopbackUrl: `ws://127.0.0.1:${port}`, diff --git a/src/commands/onboard-helpers.test.ts b/src/commands/onboard-helpers.test.ts index 1d8da5c84fb..4e50734c957 100644 --- a/src/commands/onboard-helpers.test.ts +++ b/src/commands/onboard-helpers.test.ts @@ -33,6 +33,7 @@ vi.mock("../infra/tailnet.js", () => ({ })); afterEach(() => { + vi.restoreAllMocks(); vi.unstubAllEnvs(); }); @@ -114,26 +115,30 @@ describe("resolveControlUiLinks", () => { expect(links.wsUrl).toBe("ws://127.0.0.1:18789"); }); - it("falls back to loopback when tailnet discovery throws", () => { + it("falls back to loopback for tailnet bind when interface discovery throws", () => { mocks.pickPrimaryTailnetIPv4.mockImplementationOnce(() => { throw new Error("uv_interface_addresses failed"); }); + const links = resolveControlUiLinks({ port: 18789, bind: "tailnet", }); + expect(links.httpUrl).toBe("http://127.0.0.1:18789/"); expect(links.wsUrl).toBe("ws://127.0.0.1:18789"); }); - it("falls back to loopback when LAN discovery throws", () => { - vi.spyOn(os, "networkInterfaces").mockImplementation(() => { + it("falls back to loopback for LAN bind when interface discovery throws", () => { + vi.spyOn(os, "networkInterfaces").mockImplementationOnce(() => { throw new Error("uv_interface_addresses failed"); }); + const links = resolveControlUiLinks({ port: 18789, bind: "lan", }); + expect(links.httpUrl).toBe("http://127.0.0.1:18789/"); expect(links.wsUrl).toBe("ws://127.0.0.1:18789"); }); diff --git a/src/commands/onboard-helpers.ts b/src/commands/onboard-helpers.ts index 967cfd6b5fb..e67c06ea611 100644 --- a/src/commands/onboard-helpers.ts +++ b/src/commands/onboard-helpers.ts @@ -10,9 +10,12 @@ import { resolveAgentModelPrimaryValue } from "../config/model-input.js"; import { resolveSessionTranscriptsDirForAgent } from "../config/sessions.js"; import { callGateway } from "../gateway/call.js"; import { normalizeControlUiBasePath } from "../gateway/control-ui-shared.js"; -import { pickPrimaryLanIPv4, isValidIPv4 } from "../gateway/net.js"; +import { isValidIPv4 } from "../gateway/net.js"; import { isSafeExecutableValue } from "../infra/exec-safety.js"; -import { pickPrimaryTailnetIPv4 } from "../infra/tailnet.js"; +import { + inspectBestEffortPrimaryTailnetIPv4, + pickBestEffortPrimaryLanIPv4, +} from "../infra/network-discovery-display.js"; import { isWSL } from "../infra/wsl.js"; import { runCommandWithTimeout } from "../process/exec.js"; import type { RuntimeEnv } from "../runtime.js"; @@ -456,22 +459,6 @@ function summarizeError(err: unknown): string { export const DEFAULT_WORKSPACE = DEFAULT_AGENT_WORKSPACE_DIR; -function pickPrimaryTailnetIPv4ForDisplay(): string | undefined { - try { - return pickPrimaryTailnetIPv4(); - } catch { - return undefined; - } -} - -function pickPrimaryLanIPv4ForDisplay(): string | undefined { - try { - return pickPrimaryLanIPv4(); - } catch { - return undefined; - } -} - export function resolveControlUiLinks(params: { port: number; bind?: "auto" | "lan" | "loopback" | "custom" | "tailnet"; @@ -481,7 +468,7 @@ export function resolveControlUiLinks(params: { const port = params.port; const bind = params.bind ?? "loopback"; const customBindHost = params.customBindHost?.trim(); - const tailnetIPv4 = pickPrimaryTailnetIPv4ForDisplay(); + const { tailnetIPv4 } = inspectBestEffortPrimaryTailnetIPv4(); const host = (() => { if (bind === "custom" && customBindHost && isValidIPv4(customBindHost)) { return customBindHost; @@ -490,7 +477,7 @@ export function resolveControlUiLinks(params: { return tailnetIPv4 ?? "127.0.0.1"; } if (bind === "lan") { - return pickPrimaryLanIPv4ForDisplay() ?? "127.0.0.1"; + return pickBestEffortPrimaryLanIPv4() ?? "127.0.0.1"; } return "127.0.0.1"; })(); diff --git a/src/gateway/net.test.ts b/src/gateway/net.test.ts index 255632f547d..0e0bea48d42 100644 --- a/src/gateway/net.test.ts +++ b/src/gateway/net.test.ts @@ -360,11 +360,11 @@ describe("pickPrimaryLanIPv4", () => { } }); - it("returns undefined when interface discovery throws", () => { + it("throws when interface discovery throws", () => { vi.spyOn(os, "networkInterfaces").mockImplementation(() => { throw new Error("uv_interface_addresses failed"); }); - expect(pickPrimaryLanIPv4()).toBeUndefined(); + expect(() => pickPrimaryLanIPv4()).toThrow("uv_interface_addresses failed"); }); }); diff --git a/src/gateway/net.ts b/src/gateway/net.ts index 0f4ef91a4ab..ca73bc774a0 100644 --- a/src/gateway/net.ts +++ b/src/gateway/net.ts @@ -2,7 +2,7 @@ import type { IncomingMessage } from "node:http"; import net from "node:net"; import { pickMatchingExternalInterfaceAddress, - safeNetworkInterfaces, + readNetworkInterfaces, } from "../infra/network-interfaces.js"; import { pickPrimaryTailnetIPv4, pickPrimaryTailnetIPv6 } from "../infra/tailnet.js"; import { @@ -18,7 +18,7 @@ import { * Prefers common interface names (en0, eth0) then falls back to any external IPv4. */ export function pickPrimaryLanIPv4(): string | undefined { - return pickMatchingExternalInterfaceAddress(safeNetworkInterfaces(), { + return pickMatchingExternalInterfaceAddress(readNetworkInterfaces(), { family: "IPv4", preferredNames: ["en0", "eth0"], }); diff --git a/src/infra/infra-runtime.test.ts b/src/infra/infra-runtime.test.ts index 643984f47e3..a5d18aaa784 100644 --- a/src/infra/infra-runtime.test.ts +++ b/src/infra/infra-runtime.test.ts @@ -232,13 +232,5 @@ describe("infra runtime", () => { expect(out.ipv4).toEqual(["100.123.224.76"]); expect(out.ipv6).toEqual(["fd7a:115c:a1e0::8801:e04c"]); }); - - it("returns empty address lists when interface discovery throws", () => { - vi.spyOn(os, "networkInterfaces").mockImplementation(() => { - throw new Error("uv_interface_addresses failed"); - }); - - expect(listTailnetAddresses()).toEqual({ ipv4: [], ipv6: [] }); - }); }); }); diff --git a/src/infra/network-discovery-display.test.ts b/src/infra/network-discovery-display.test.ts new file mode 100644 index 00000000000..19fc78ecd1d --- /dev/null +++ b/src/infra/network-discovery-display.test.ts @@ -0,0 +1,67 @@ +import os from "node:os"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { makeNetworkInterfacesSnapshot } from "../test-helpers/network-interfaces.js"; +import { + inspectBestEffortPrimaryTailnetIPv4, + pickBestEffortPrimaryLanIPv4, + resolveBestEffortGatewayBindHostForDisplay, +} from "./network-discovery-display.js"; + +describe("network display discovery", () => { + afterEach(() => { + vi.restoreAllMocks(); + }); + + it("returns no LAN address when interface discovery throws", () => { + vi.spyOn(os, "networkInterfaces").mockImplementation(() => { + throw new Error("uv_interface_addresses failed"); + }); + + expect(pickBestEffortPrimaryLanIPv4()).toBeUndefined(); + }); + + it("reports a warning when tailnet inspection throws", () => { + vi.spyOn(os, "networkInterfaces").mockImplementation(() => { + throw new Error("uv_interface_addresses failed"); + }); + + expect( + inspectBestEffortPrimaryTailnetIPv4({ + warningPrefix: "Status could not inspect tailnet addresses", + }), + ).toEqual({ + tailnetIPv4: undefined, + warning: "Status could not inspect tailnet addresses: uv_interface_addresses failed.", + }); + }); + + it("falls back to loopback when bind host resolution throws", async () => { + vi.spyOn(os, "networkInterfaces").mockImplementation(() => { + throw new Error("uv_interface_addresses failed"); + }); + + await expect( + resolveBestEffortGatewayBindHostForDisplay({ + bindMode: "tailnet", + warningPrefix: + "Status is using fallback network details because interface discovery failed", + }), + ).resolves.toEqual({ + bindHost: "127.0.0.1", + warning: + "Status is using fallback network details because interface discovery failed: uv_interface_addresses failed.", + }); + }); + + it("still returns discovered tailnet values when interfaces are available", () => { + vi.spyOn(os, "networkInterfaces").mockReturnValue( + makeNetworkInterfacesSnapshot({ + utun9: [{ address: "100.88.1.5", family: "IPv4" }], + }), + ); + + expect(inspectBestEffortPrimaryTailnetIPv4()).toEqual({ + tailnetIPv4: "100.88.1.5", + }); + }); +}); diff --git a/src/infra/network-discovery-display.ts b/src/infra/network-discovery-display.ts new file mode 100644 index 00000000000..af2ba818213 --- /dev/null +++ b/src/infra/network-discovery-display.ts @@ -0,0 +1,66 @@ +import type { GatewayBindMode } from "../config/types.js"; +import { pickPrimaryLanIPv4, resolveGatewayBindHost } from "../gateway/net.js"; +import { pickPrimaryTailnetIPv4 } from "./tailnet.js"; + +export function summarizeDisplayNetworkError(error: unknown): string { + if (error instanceof Error) { + const message = error.message.trim(); + if (message) { + return message; + } + } + return "network interface discovery failed"; +} + +export function fallbackBindHostForDisplay( + bindMode: GatewayBindMode, + customBindHost?: string, +): string { + if (bindMode === "lan") { + return "0.0.0.0"; + } + if (bindMode === "custom") { + return customBindHost?.trim() || "0.0.0.0"; + } + return "127.0.0.1"; +} + +export function pickBestEffortPrimaryLanIPv4(): string | undefined { + try { + return pickPrimaryLanIPv4(); + } catch { + return undefined; + } +} + +export function inspectBestEffortPrimaryTailnetIPv4(params?: { warningPrefix?: string }): { + tailnetIPv4: string | undefined; + warning?: string; +} { + try { + return { tailnetIPv4: pickPrimaryTailnetIPv4() }; + } catch (error) { + const prefix = params?.warningPrefix?.trim(); + const warning = prefix ? `${prefix}: ${summarizeDisplayNetworkError(error)}.` : undefined; + return { tailnetIPv4: undefined, ...(warning ? { warning } : {}) }; + } +} + +export async function resolveBestEffortGatewayBindHostForDisplay(params: { + bindMode: GatewayBindMode; + customBindHost?: string; + warningPrefix?: string; +}): Promise<{ bindHost: string; warning?: string }> { + try { + return { + bindHost: await resolveGatewayBindHost(params.bindMode, params.customBindHost), + }; + } catch (error) { + const prefix = params.warningPrefix?.trim(); + const warning = prefix ? `${prefix}: ${summarizeDisplayNetworkError(error)}.` : undefined; + return { + bindHost: fallbackBindHostForDisplay(params.bindMode, params.customBindHost), + ...(warning ? { warning } : {}), + }; + } +} diff --git a/src/infra/network-interfaces.ts b/src/infra/network-interfaces.ts index cfaf95f6979..4b1329ec4ad 100644 --- a/src/infra/network-interfaces.ts +++ b/src/infra/network-interfaces.ts @@ -20,11 +20,17 @@ function normalizeNetworkInterfaceFamily( return undefined; } +export function readNetworkInterfaces( + networkInterfaces: () => NetworkInterfacesSnapshot = os.networkInterfaces, +): NetworkInterfacesSnapshot { + return networkInterfaces(); +} + export function safeNetworkInterfaces( networkInterfaces: () => NetworkInterfacesSnapshot = os.networkInterfaces, ): NetworkInterfacesSnapshot | undefined { try { - return networkInterfaces(); + return readNetworkInterfaces(networkInterfaces); } catch { return undefined; } diff --git a/src/infra/system-presence.ts b/src/infra/system-presence.ts index b11cd288cf8..e8b4c394f28 100644 --- a/src/infra/system-presence.ts +++ b/src/infra/system-presence.ts @@ -1,7 +1,7 @@ import { spawnSync } from "node:child_process"; import os from "node:os"; -import { pickPrimaryLanIPv4 } from "../gateway/net.js"; import { resolveRuntimeServiceVersion } from "../version.js"; +import { pickBestEffortPrimaryLanIPv4 } from "./network-discovery-display.js"; export type SystemPresence = { host?: string; @@ -45,12 +45,7 @@ function normalizePresenceKey(key: string | undefined): string | undefined { } function resolvePrimaryIPv4(): string | undefined { - const host = os.hostname(); - try { - return pickPrimaryLanIPv4() ?? host; - } catch { - return host; - } + return pickBestEffortPrimaryLanIPv4() ?? os.hostname(); } function initSelfPresence() { diff --git a/src/infra/tailnet.test.ts b/src/infra/tailnet.test.ts index 446d45dec5b..dfe9adc0937 100644 --- a/src/infra/tailnet.test.ts +++ b/src/infra/tailnet.test.ts @@ -54,4 +54,12 @@ describe("tailnet helpers", () => { expect(pickPrimaryTailnetIPv4()).toBe("100.99.1.1"); expect(pickPrimaryTailnetIPv6()).toBe("fd7a:115c:a1e0::9"); }); + + it("throws when interface discovery fails", () => { + vi.spyOn(os, "networkInterfaces").mockImplementation(() => { + throw new Error("uv_interface_addresses failed"); + }); + + expect(() => listTailnetAddresses()).toThrow("uv_interface_addresses failed"); + }); }); diff --git a/src/infra/tailnet.ts b/src/infra/tailnet.ts index bef460caffb..c8ab1d2bfe7 100644 --- a/src/infra/tailnet.ts +++ b/src/infra/tailnet.ts @@ -1,5 +1,5 @@ import { isIpInCidr } from "../shared/net/ip.js"; -import { listExternalInterfaceAddresses, safeNetworkInterfaces } from "./network-interfaces.js"; +import { listExternalInterfaceAddresses, readNetworkInterfaces } from "./network-interfaces.js"; export type TailnetAddresses = { ipv4: string[]; @@ -25,7 +25,7 @@ export function listTailnetAddresses(): TailnetAddresses { const ipv4: string[] = []; const ipv6: string[] = []; - for (const { address, family } of listExternalInterfaceAddresses(safeNetworkInterfaces())) { + for (const { address, family } of listExternalInterfaceAddresses(readNetworkInterfaces())) { if (family === "IPv4" && isTailnetIPv4(address)) { ipv4.push(address); }