From f5fe05fe16fdfef042c430126abb4a7b19d73072 Mon Sep 17 00:00:00 2001 From: Jared Ping <13906915+JaredP94@users.noreply.github.com> Date: Tue, 3 Nov 2026 00:09:07 +0200 Subject: [PATCH 01/10] test(services): isolate cross-process state and local fixtures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Port NVIDIA/Personal-AI-Router@937cf6c to this fork’s standalone proxies. Route every runtime command through a per-test app-data environment, retain explicit fixture roots for restarts and remote engine manifests, and guard against bypassing the command factory or replacing its environment. Keep Go build caches outside disposable application state. Own the manual bridge tests’ node-info fixture instead of inheriting a running PAIR instance’s cluster identity. Check wildcard and IPv6 listeners before tests require fixed worker ports, including the workload and cluster restart cases, so occupied developer ports are reported as skips rather than misleading product failures. Co-authored-by: Som Signed-off-by: Jared Ping <13906915+JaredP94@users.noreply.github.com> --- services/readme.md | 13 +- services/tests/broker_management_test.go | 5 + services/tests/broker_supervision_test.go | 68 ++++--- services/tests/broker_test.go | 3 +- services/tests/cluster_manager_test.go | 6 +- services/tests/cluster_restore_test.go | 12 +- services/tests/errors_test.go | 3 +- services/tests/ghost_node_test.go | 3 +- services/tests/isolation_test.go | 182 ++++++++++++++++++ services/tests/log_level_test.go | 7 +- services/tests/main_test.go | 36 ++++ services/tests/node_settings_test.go | 3 +- services/tests/protocol_v2_test.go | 7 +- services/tests/remote_engine_test.go | 7 +- services/tests/scheduler_interop_test.go | 3 +- services/tests/secure_inference_test.go | 6 +- services/tests/tls_test.go | 3 +- .../tests/workload_identity_interop_test.go | 8 +- services/tests/workload_interop_test.go | 8 +- 19 files changed, 301 insertions(+), 82 deletions(-) create mode 100644 services/tests/isolation_test.go diff --git a/services/readme.md b/services/readme.md index 955c7404e..1c49f8592 100644 --- a/services/readme.md +++ b/services/readme.md @@ -280,8 +280,19 @@ cd tests go test ./... ``` +Every runtime launch uses a per-test temporary app-data root for `HOME`, +`USERPROFILE`, `XDG_CONFIG_HOME`, `APPDATA`, and `LOCALAPPDATA`, inherited by +supervised workers. Restart tests explicitly reuse their own temporary root. +Runtime commands inherit the remaining environment with those five roots +overridden; build commands keep the original home/config roots so Go's caches +are not redirected into disposable test state. The +`TestProcessHelpersRequireIsolation` source guard rejects launch helpers that +bypass the isolated command factory or replace its environment. + Expect this to take a few minutes. Run it before opening a merge request, and -after any change to a JSON-RPC method, payload, or notification. +after any change to a JSON-RPC method, payload, or notification. CI runs the +cross-process suite on macOS arm64; component vet/tests run on both macOS and +Linux. ### Tests that skip themselves diff --git a/services/tests/broker_management_test.go b/services/tests/broker_management_test.go index 3342878ed..967165749 100644 --- a/services/tests/broker_management_test.go +++ b/services/tests/broker_management_test.go @@ -149,6 +149,10 @@ func TestBrokerBridgesManualNodeIntoProxy(t *testing.T) { stopOllama := fakeOllama(t) // skips if 11434 unavailable t.Cleanup(stopOllama) + // Own node-info as well as the engine: a running local PAIR must not + // supply its cluster principal/UUID to this deliberately unclustered node. + t.Cleanup(fakeNodeInfo(t, "")) + stdin, msgs, _, cleanup := startBrokerWith(t, "--manual-nodes-path", manualNodesBin, "--proxy-path", proxyBin, @@ -257,6 +261,7 @@ func TestBrokerBridgesManualNodeIntoLMStudioProxy(t *testing.T) { configDir := persistLMStudioProxyPort(t, freePort(t)) stopLM := fakeLMStudio(t) // skips if 1234 unavailable t.Cleanup(stopLM) + t.Cleanup(fakeNodeInfo(t, "")) stdin, msgs, _, cleanup := startBrokerWithConfigDir(t, configDir, "--manual-nodes-path", manualNodesBin, diff --git a/services/tests/broker_supervision_test.go b/services/tests/broker_supervision_test.go index 03266b25b..07f5db174 100644 --- a/services/tests/broker_supervision_test.go +++ b/services/tests/broker_supervision_test.go @@ -15,7 +15,6 @@ import ( "io" "net" "os" - "os/exec" "path/filepath" "regexp" "runtime" @@ -75,18 +74,9 @@ func startBrokerWithDirsAndEnv(t *testing.T, configDir, clusterDir string, extra func startSupervisedBrokerInDir(t *testing.T, workDir, configDir, clusterDir string, extraEnv []string, extraArgs ...string) (stdin io.WriteCloser, msgs <-chan jsonrpc.Message, stderrLines <-chan string, cleanup func()) { t.Helper() args := append([]string{"--scanner-path", scannerBin, "--cluster-dir", clusterDir}, extraArgs...) - cmd := exec.Command(brokerBin, args...) + cmd := testCommandInDir(t, configDir, brokerBin, args...) cmd.Dir = workDir - // Every supervised worker must use disposable per-test config. In - // particular, managed-port startup can persist an Ollama backend port; a - // cross-process test must never read or rewrite the developer's real file. - cmd.Env = append(os.Environ(), - "HOME="+configDir, - "XDG_CONFIG_HOME="+configDir, - "APPDATA="+configDir, - "LOCALAPPDATA="+configDir, - ) - cmd.Env = append(cmd.Env, extraEnv...) + addTestEnv(t, cmd, extraEnv...) stdinPipe, err := cmd.StdinPipe() if err != nil { @@ -199,19 +189,42 @@ const methodErrorsUpdate = "errors:update" var proxyPidRe = regexp.MustCompile(`proxy started.*\bpid=(\d+)`) -// portBusy reports whether a TCP port on localhost is already bound — used +// portBusy probes the same wildcard address the workers bind, not just IPv4 +// loopback (which can coexist with an IPv6 listener on macOS). It is used // to skip tests whose supervised workers need a fatal-on-conflict listener // (nvpair-errors --peer-sync on 14319, ollama-proxy on 11435) when something // else on the host already holds it. func portBusy(port int) bool { - ln, err := net.Listen("tcp", fmt.Sprintf("127.0.0.1:%d", port)) - if err != nil { - return true + for _, host := range []string{"", "127.0.0.1", "::1"} { + if host == "::1" { + // An unavailable IPv6 loopback is not an occupied port. Probe + // support independently before interpreting a bind failure. + probe, err := net.Listen("tcp6", "[::1]:0") + if err != nil { + continue + } + probe.Close() + } + ln, err := net.Listen("tcp", net.JoinHostPort(host, strconv.Itoa(port))) + if err != nil { + return true + } + ln.Close() } - ln.Close() return false } +func TestPortBusyDetectsIPv6Listener(t *testing.T) { + ln, err := net.Listen("tcp6", "[::1]:0") + if err != nil { + t.Skipf("IPv6 unavailable: %v", err) + } + defer ln.Close() + if !portBusy(ln.Addr().(*net.TCPAddr).Port) { + t.Fatal("IPv6 listener was missed by the worker-port preflight") + } +} + // TestBrokerCrashSurfacingAndRestart kills the broker's supervised proxy // and asserts the crash is surfaced as supervisor:subprocess-crashed:proxy // through the nvpair-errors pipeline (errors:update relay + errors:get-initial), @@ -291,7 +304,7 @@ func TestBrokerShutsDownOnSignal(t *testing.T) { // other tests' teardown already exercises. t.Skip("signalling a specific process is not supported on Windows") } - cmd := exec.Command(brokerBin, + cmd := testCommand(t, brokerBin, "--scanner-path", scannerBin, "--proxy-path", proxyBin, "--cluster-dir", t.TempDir(), @@ -424,20 +437,11 @@ func TestBrokerProxySetPortRebinds(t *testing.T) { if portBusy(11435) { t.Skip("ollama-proxy default port 11435 already in use; skipping") } - cfg := t.TempDir() - cmd := exec.Command(brokerBin, + cmd := testCommand(t, brokerBin, "--scanner-path", scannerBin, "--proxy-path", proxyBin, "--engine-manager-path", engineMgrBin, ) - // Isolate per-user state (proxy-port.json, any engine override) to a - // temp dir so the test leaves the real config untouched. - cmd.Env = append(os.Environ(), - "HOME="+cfg, - "XDG_CONFIG_HOME="+cfg, - "APPDATA="+cfg, - "LOCALAPPDATA="+cfg, - ) cmd.Stderr = os.Stderr stdin, err := cmd.StdinPipe() if err != nil { @@ -583,7 +587,7 @@ func waitWorkerRunning(t *testing.T, stdin io.Writer, msgs <-chan jsonrpc.Messag func signalWorker(t *testing.T, pid int, signal string) { t.Helper() - if output, err := exec.Command("kill", "-"+signal, strconv.Itoa(pid)).CombinedOutput(); err != nil { + if output, err := testCommand(t, "kill", "-"+signal, strconv.Itoa(pid)).CombinedOutput(); err != nil { t.Fatalf("kill -%s %d: %v %s", signal, pid, err, output) } } @@ -597,7 +601,7 @@ func TestHungSchedulerRestarts(t *testing.T) { waitForMethod(t, msgs, "app:ready", 10*time.Second) old := waitWorkerRunning(t, stdin, msgs, "scheduler").PID signalWorker(t, old, "STOP") - t.Cleanup(func() { _ = exec.Command("kill", "-CONT", strconv.Itoa(old)).Run() }) + t.Cleanup(func() { _ = testCommand(t, "kill", "-CONT", strconv.Itoa(old)).Run() }) waitWorkerState(t, msgs, "scheduler", "degraded", 8*time.Second) waitWorkerState(t, msgs, "scheduler", "restarting", 10*time.Second) status := waitWorkerState(t, msgs, "scheduler", "running", 5*time.Second) @@ -609,7 +613,7 @@ func TestHungSchedulerRestarts(t *testing.T) { tick := time.NewTicker(20 * time.Millisecond) defer tick.Stop() for { - if exec.Command("kill", "-0", strconv.Itoa(old)).Run() != nil { + if testCommand(t, "kill", "-0", strconv.Itoa(old)).Run() != nil { break } select { @@ -632,7 +636,7 @@ func TestHungEngineManagerOnlyDegrades(t *testing.T) { Workers []workerHealthStatus `json:"workers"` } signalWorker(t, pid, "STOP") - t.Cleanup(func() { _ = exec.Command("kill", "-CONT", strconv.Itoa(pid)).Run() }) + t.Cleanup(func() { _ = testCommand(t, "kill", "-CONT", strconv.Itoa(pid)).Run() }) waitWorkerState(t, msgs, "engine-manager", "degraded", 8*time.Second) timer := time.NewTimer(10 * time.Second) defer timer.Stop() diff --git a/services/tests/broker_test.go b/services/tests/broker_test.go index 22944f5b2..235a5b565 100644 --- a/services/tests/broker_test.go +++ b/services/tests/broker_test.go @@ -27,7 +27,6 @@ import ( "fmt" "io" "os" - "os/exec" "testing" "time" @@ -59,7 +58,7 @@ type subscriptionResult struct { func startBroker(t *testing.T) (stdin io.WriteCloser, msgs <-chan jsonrpc.Message, cleanup func()) { t.Helper() - cmd := exec.Command(brokerBin, "--scanner-path", scannerBin, "--cluster-dir", t.TempDir()) + cmd := testCommand(t, brokerBin, "--scanner-path", scannerBin, "--cluster-dir", t.TempDir()) cmd.Stderr = os.Stderr stdinPipe, err := cmd.StdinPipe() diff --git a/services/tests/cluster_manager_test.go b/services/tests/cluster_manager_test.go index bec9cb999..704304423 100644 --- a/services/tests/cluster_manager_test.go +++ b/services/tests/cluster_manager_test.go @@ -36,10 +36,8 @@ func startCM(t *testing.T, configDir string, port int) *cmProc { // cancel-vs-Completion commit-window delay). func startCMEnv(t *testing.T, configDir string, port int, extraEnv []string) *cmProc { t.Helper() - cmd := exec.Command(clusterMgrBin, "--config-dir", configDir, "--port", strconv.Itoa(port)) - if len(extraEnv) > 0 { - cmd.Env = append(os.Environ(), extraEnv...) - } + cmd := testCommand(t, clusterMgrBin, "--config-dir", configDir, "--port", strconv.Itoa(port)) + addTestEnv(t, cmd, extraEnv...) stdin, err := cmd.StdinPipe() if err != nil { t.Fatalf("stdin pipe: %v", err) diff --git a/services/tests/cluster_restore_test.go b/services/tests/cluster_restore_test.go index a8d37123c..e5261c47b 100644 --- a/services/tests/cluster_restore_test.go +++ b/services/tests/cluster_restore_test.go @@ -13,7 +13,6 @@ import ( "fmt" "io" "os" - "os/exec" "testing" "time" @@ -26,13 +25,7 @@ import ( func startBrokerInDir(t *testing.T, configDir string, extraArgs ...string) (io.WriteCloser, <-chan jsonrpc.Message, func()) { t.Helper() args := append([]string{"--scanner-path", scannerBin, "--cluster-dir", t.TempDir()}, extraArgs...) - cmd := exec.Command(brokerBin, args...) - cmd.Env = append(os.Environ(), - "HOME="+configDir, - "XDG_CONFIG_HOME="+configDir, - "APPDATA="+configDir, - "LOCALAPPDATA="+configDir, - ) + cmd := testCommandInDir(t, configDir, brokerBin, args...) stdin, err := cmd.StdinPipe() if err != nil { t.Fatalf("broker stdin pipe: %v", err) @@ -85,6 +78,9 @@ func waitForResponseID(t *testing.T, msgs <-chan jsonrpc.Message, id int, timeou // config dir, and asserts cluster:get-node-id reports the restored clusterId // (rather than the "" a fresh cluster-manager comes up with). func TestBrokerRestoresClusterIdentityAfterRestart(t *testing.T) { + if portBusy(14321) { + t.Skip("cluster-manager inter-node port 14321 already in use; skipping") + } configDir := t.TempDir() const clusterID = "restore-test-cluster-0001" const friendly = "Restore Test Lab" diff --git a/services/tests/errors_test.go b/services/tests/errors_test.go index 16d291a44..0e9a16bc7 100644 --- a/services/tests/errors_test.go +++ b/services/tests/errors_test.go @@ -9,7 +9,6 @@ import ( "encoding/json" "io" "os" - "os/exec" "sort" "strconv" "sync" @@ -36,7 +35,7 @@ import ( // - clear → update broadcast with empty list // - re-report after clear → resurrect (ack-until-reemit) func TestErrorsBrokerPipeline_EndToEnd(t *testing.T) { - cmd := exec.Command(errorsBin) + cmd := testCommand(t, errorsBin) stdin, err := cmd.StdinPipe() if err != nil { t.Fatalf("stdin pipe: %v", err) diff --git a/services/tests/ghost_node_test.go b/services/tests/ghost_node_test.go index 1cff4d68c..31042c890 100644 --- a/services/tests/ghost_node_test.go +++ b/services/tests/ghost_node_test.go @@ -10,7 +10,6 @@ import ( "net/http" "net/http/httptest" "os" - "os/exec" "strconv" "strings" "sync/atomic" @@ -129,7 +128,7 @@ func TestScannerEvictsRecordSupersededAtItsAddress(t *testing.T) { // real cluster identity on the LAN while the test runs. func startScannerForGhostTest(t *testing.T) <-chan jsonrpc.Message { t.Helper() - cmd := exec.Command(scannerBin, "--cluster-dir", t.TempDir()) + cmd := testCommand(t, scannerBin, "--cluster-dir", t.TempDir()) cmd.Stderr = os.Stderr stdin, err := cmd.StdinPipe() diff --git a/services/tests/isolation_test.go b/services/tests/isolation_test.go new file mode 100644 index 000000000..192b70192 --- /dev/null +++ b/services/tests/isolation_test.go @@ -0,0 +1,182 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +package tests + +import ( + "encoding/json" + "go/ast" + "go/parser" + "go/token" + "os" + "path/filepath" + "runtime" + "strings" + "testing" + "time" +) + +// TestProcessHelpersRequireIsolation prevents new launch helpers from bypassing +// the sole runtime command constructor. goBuild deliberately keeps the real Go +// cache environment: it builds, but never executes, an application binary. +func TestProcessHelpersRequireIsolation(t *testing.T) { + files, err := filepath.Glob("*.go") + if err != nil { + t.Fatal(err) + } + fset := token.NewFileSet() + for _, path := range files { + file, err := parser.ParseFile(fset, path, nil, 0) + if err != nil { + t.Fatal(err) + } + execNames := map[string]bool{} + osNames := map[string]bool{} + for _, imp := range file.Imports { + if imp.Path.Value == `"os"` { + name := "os" + if imp.Name != nil { + name = imp.Name.Name + } + if name == "." { + t.Errorf("%s: dot import bypasses isolation guard", path) + } + osNames[name] = true + } + if imp.Path.Value == `"os/exec"` { + name := "exec" + if imp.Name != nil { + name = imp.Name.Name + } + if name == "." { + t.Errorf("%s: dot import bypasses isolation guard", path) + } + execNames[name] = true + } + } + // *exec.Cmd annotations are harmless; values and constructors must + // come from the isolated factory, including new(exec.Cmd) or literals. + pointerTypes := map[*ast.SelectorExpr]bool{} + ast.Inspect(file, func(node ast.Node) bool { + if ptr, ok := node.(*ast.StarExpr); ok { + if sel, ok := ptr.X.(*ast.SelectorExpr); ok { + pointerTypes[sel] = true + } + } + return true + }) + for _, decl := range file.Decls { + fn, isFunc := decl.(*ast.FuncDecl) + allowed := isFunc && path == "main_test.go" && (fn.Name.Name == "goBuild" || fn.Name.Name == "testCommandInDir") + envAllowed := allowed || (isFunc && path == "main_test.go" && fn.Name.Name == "addTestEnv") + ast.Inspect(decl, func(node ast.Node) bool { + if assign, ok := node.(*ast.AssignStmt); ok && !envAllowed { + for _, lhs := range assign.Lhs { + if sel, ok := lhs.(*ast.SelectorExpr); ok && sel.Sel.Name == "Env" { + t.Errorf("%s: use addTestEnv instead of replacing isolated Env", fset.Position(sel.Pos())) + } + } + } + sel, ok := node.(*ast.SelectorExpr) + if !ok { + return true + } + pkg, ok := sel.X.(*ast.Ident) + if ok && osNames[pkg.Name] && sel.Sel.Name == "StartProcess" { + t.Errorf("%s: use testCommand instead of os.StartProcess", fset.Position(sel.Pos())) + } + if ok && execNames[pkg.Name] && !pointerTypes[sel] && !allowed { + t.Errorf("%s: use testCommand/testCommandInDir instead of os/exec.%s", fset.Position(sel.Pos()), sel.Sel.Name) + } + return true + }) + } + } +} + +// Ported from NVIDIA/Personal-AI-Router@937cf6c, using this fork's +// standalone LM Studio proxy rather than the upstream unified proxy facade. +func TestLMStudioChildPersistsUnderPrivateBase(t *testing.T) { + root := t.TempDir() + cmd := testCommandInDir(t, root, lmstudioProxyBin, "--port", "0", "--ignore-persisted-port") + stdin, err := cmd.StdinPipe() + if err != nil { + t.Fatal(err) + } + stdout, err := cmd.StdoutPipe() + if err != nil { + t.Fatal(err) + } + if err := cmd.Start(); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = stdin.Close(); _ = cmd.Process.Kill(); _ = cmd.Wait() }) + msgs := startMsgReader(stdout) + waitForMethod(t, msgs, "ready", 10*time.Second) + var port int + for attempt := 0; attempt < 8; attempt++ { + port = freePort(t) + sendLine(t, stdin, map[string]any{ + "jsonrpc": "2.0", "id": attempt + 1, "method": "set-port", + "params": map[string]int{"port": port}, + }) + resp := waitForResponse(t, msgs, 5*time.Second) + if resp.Error == nil { + break + } + if !strings.HasPrefix(resp.Error.Message, "failed to bind port") { + t.Fatalf("set-port: %+v", resp.Error) + } + if attempt == 7 { + t.Fatal("all probed ports were taken before bind") + } + } + base := root + if runtime.GOOS == "darwin" { + base = filepath.Join(root, "Library", "Application Support") + } + path := filepath.Join(base, "Nvidia Corporation", "Personal AI Router", "lmstudio-proxy-port.json") + data, err := os.ReadFile(path) + if err != nil { + t.Fatalf("child did not persist inside its private root: %v", err) + } + var saved struct { + Port int `json:"port"` + } + if err := json.Unmarshal(data, &saved); err != nil { + t.Fatal(err) + } + if saved.Port != port { + t.Fatalf("saved port = %d, want %d", saved.Port, port) + } +} + +func TestCommandEnvironmentIsPerTest(t *testing.T) { + var roots []string + for _, name := range []string{"first", "second"} { + t.Run(name, func(t *testing.T) { + cmd := testCommand(t, "not-executed") + values := map[string]string{} + for _, entry := range cmd.Environ() { + key, value, _ := strings.Cut(entry, "=") + values[key] = value + } + root := values["HOME"] + if root == "" || root == os.Getenv("HOME") { + t.Fatal("HOME is not isolated") + } + if _, err := os.Stat(root); err != nil { + t.Fatal(err) + } + for _, key := range []string{"HOME", "XDG_CONFIG_HOME", "APPDATA", "LOCALAPPDATA", "USERPROFILE"} { + if values[key] != root { + t.Errorf("%s = %q, want %q", key, values[key], root) + } + } + roots = append(roots, root) + }) + } + if len(roots) == 2 && roots[0] == roots[1] { + t.Fatal("tests share an app-data root") + } +} diff --git a/services/tests/log_level_test.go b/services/tests/log_level_test.go index c1a1dfb0d..452601de4 100644 --- a/services/tests/log_level_test.go +++ b/services/tests/log_level_test.go @@ -8,7 +8,6 @@ import ( "bytes" "encoding/json" "io" - "os/exec" "strings" "sync" "testing" @@ -47,7 +46,7 @@ func (b *safeBuffer) Reset() { func startProxyWithLog(t *testing.T, level string) (stdin io.WriteCloser, msgs <-chan jsonrpc.Message, stderr *safeBuffer, cleanup func()) { t.Helper() - cmd := exec.Command(proxyBin, "--log-level", level) + cmd := testCommand(t, proxyBin, "--log-level", level) stderrBuf := &safeBuffer{} cmd.Stderr = stderrBuf @@ -185,8 +184,8 @@ func TestLogSetLevelViaRPC(t *testing.T) { // TestLogLevelEnvFallback verifies NVPAIR_LOG_LEVEL is honoured when --log-level // is not passed. func TestLogLevelEnvFallback(t *testing.T) { - cmd := exec.Command(proxyBin) - cmd.Env = append(cmd.Environ(), "NVPAIR_LOG_LEVEL=debug") + cmd := testCommand(t, proxyBin) + addTestEnv(t, cmd, "NVPAIR_LOG_LEVEL=debug") stderrBuf := &safeBuffer{} cmd.Stderr = stderrBuf diff --git a/services/tests/main_test.go b/services/tests/main_test.go index 52171a1cc..7b5f76e2d 100644 --- a/services/tests/main_test.go +++ b/services/tests/main_test.go @@ -12,6 +12,7 @@ import ( "os/exec" "path/filepath" "runtime" + "strings" "testing" "time" @@ -164,6 +165,41 @@ func goBuild(srcDir, output string) error { return cmd.Run() } +var isolatedHomeKeys = []string{"HOME", "XDG_CONFIG_HOME", "APPDATA", "LOCALAPPDATA", "USERPROFILE"} + +// testCommand is the only runtime command factory. Like upstream's post-build +// isolation, this leaves goBuild's module/build caches alone, but gives each +// test child (and all its supervised workers) disposable app-data state. +func testCommand(t *testing.T, binary string, args ...string) *exec.Cmd { + t.Helper() + return testCommandInDir(t, t.TempDir(), binary, args...) +} + +// An explicit root lets restart tests reuse their own persisted state. +func testCommandInDir(t *testing.T, root, binary string, args ...string) *exec.Cmd { + t.Helper() + cmd := exec.Command(binary, args...) + cmd.Env = os.Environ() + for _, key := range isolatedHomeKeys { + cmd.Env = append(cmd.Env, key+"="+root) + } + return cmd +} + +// addTestEnv permits scenario-specific overrides without undoing isolation. +func addTestEnv(t *testing.T, cmd *exec.Cmd, entries ...string) { + t.Helper() + for _, entry := range entries { + key, _, _ := strings.Cut(entry, "=") + for _, isolated := range isolatedHomeKeys { + if strings.EqualFold(key, isolated) { + t.Fatalf("test environment cannot override %s", isolated) + } + } + } + cmd.Env = append(cmd.Env, entries...) +} + // --- JSON-RPC helpers --- // The on-wire JSON-RPC frame and error envelope are the shared diff --git a/services/tests/node_settings_test.go b/services/tests/node_settings_test.go index 77221a64e..bd22e0319 100644 --- a/services/tests/node_settings_test.go +++ b/services/tests/node_settings_test.go @@ -7,7 +7,6 @@ import ( "encoding/json" "io" "os" - "os/exec" "path/filepath" "strconv" "sync/atomic" @@ -35,7 +34,7 @@ import ( func startNodeSettings(t *testing.T, settingsPath string) (io.WriteCloser, <-chan jsonrpc.Message, func()) { t.Helper() - cmd := exec.Command(nodeSettingsBin, "--settings", settingsPath) + cmd := testCommand(t, nodeSettingsBin, "--settings", settingsPath) cmd.Stderr = os.Stderr stdinPipe, err := cmd.StdinPipe() diff --git a/services/tests/protocol_v2_test.go b/services/tests/protocol_v2_test.go index 7969a324b..36fb250c5 100644 --- a/services/tests/protocol_v2_test.go +++ b/services/tests/protocol_v2_test.go @@ -9,7 +9,6 @@ import ( "encoding/json" "errors" "io" - "os" "os/exec" "strings" "testing" @@ -40,15 +39,11 @@ type protocolV2Broker struct { func startProtocolV2Broker(t *testing.T) *protocolV2Broker { t.Helper() p := &protocolV2Broker{ - cmd: exec.Command(brokerBin, "--scanner-path", scannerBin, "--cluster-dir", t.TempDir()), + cmd: testCommand(t, brokerBin, "--scanner-path", scannerBin, "--cluster-dir", t.TempDir()), messages: make(chan jsonrpc.Message, 64), readError: make(chan error, 1), done: make(chan struct{}), } // Optional binaries resolve ONLY against cwd, not next to brokerBin. p.cmd.Dir = t.TempDir() - p.cmd.Env = os.Environ() - for _, key := range []string{"HOME", "XDG_CONFIG_HOME", "APPDATA", "LOCALAPPDATA"} { - p.cmd.Env = append(p.cmd.Env, key+"="+t.TempDir()) - } p.cmd.Stderr = &p.stderr p.cmd.WaitDelay = time.Second var err error diff --git a/services/tests/remote_engine_test.go b/services/tests/remote_engine_test.go index 07b3af724..c029ddd16 100644 --- a/services/tests/remote_engine_test.go +++ b/services/tests/remote_engine_test.go @@ -23,7 +23,6 @@ import ( "math/big" "net/url" "os" - "os/exec" "path/filepath" "reflect" "runtime" @@ -202,7 +201,7 @@ func TestRemoteEngineGetInstalled(t *testing.T) { // would leak into the fixture. func setRemoteEngineHome(t *testing.T, dir string) { t.Helper() - for _, key := range []string{"HOME", "XDG_CONFIG_HOME", "APPDATA", "LOCALAPPDATA"} { + for _, key := range []string{"HOME", "XDG_CONFIG_HOME", "APPDATA", "LOCALAPPDATA", "USERPROFILE"} { t.Setenv(key, dir) } } @@ -247,7 +246,7 @@ func TestRemoteEngineRejectsUntrusted(t *testing.T) { // stays alive) and drains stdout. Returns stdin and a cleanup. func startEngineManagerServer(t *testing.T, clusterDir string, controlPort int) (io.WriteCloser, func()) { t.Helper() - cmd := exec.Command(engineMgrBin, + cmd := testCommandInDir(t, clusterDir, engineMgrBin, "--control-port", fmt.Sprintf("%d", controlPort), "--cluster-dir", clusterDir, "--log-level", "warn", @@ -282,7 +281,7 @@ func startEngineManagerServer(t *testing.T, clusterDir string, controlPort int) // client. Returns its stdin, a reader over its stdout frames, and a cleanup. func startEngineManagerStdio(t *testing.T, clusterDir string) (io.WriteCloser, <-chan jsonrpc.Message, func()) { t.Helper() - cmd := exec.Command(engineMgrBin, "--cluster-dir", clusterDir, "--log-level", "warn") + cmd := testCommandInDir(t, clusterDir, engineMgrBin, "--cluster-dir", clusterDir, "--log-level", "warn") cmd.Stderr = os.Stderr stdin, err := cmd.StdinPipe() if err != nil { diff --git a/services/tests/scheduler_interop_test.go b/services/tests/scheduler_interop_test.go index 267c82504..89855486a 100644 --- a/services/tests/scheduler_interop_test.go +++ b/services/tests/scheduler_interop_test.go @@ -22,7 +22,6 @@ import ( "net/http" "net/http/httptest" "os" - "os/exec" "sort" "sync" "testing" @@ -57,7 +56,7 @@ func writeStampedSchedulerNotification(t *testing.T, w io.Writer, nextSeq *int, // reader over its stdout JSON-RPC frames, and a cleanup. func startSchedulerProc(t *testing.T, args ...string) (io.WriteCloser, <-chan jsonrpc.Message, func()) { t.Helper() - cmd := exec.Command(schedulerBin, args...) + cmd := testCommand(t, schedulerBin, args...) cmd.Stderr = os.Stderr stdin, err := cmd.StdinPipe() if err != nil { diff --git a/services/tests/secure_inference_test.go b/services/tests/secure_inference_test.go index f2f3e49f6..7a3aef0f8 100644 --- a/services/tests/secure_inference_test.go +++ b/services/tests/secure_inference_test.go @@ -62,15 +62,11 @@ type proxyProc struct { // config. func startProxyProc(t *testing.T, clusterDir string, listenPort int) *proxyProc { t.Helper() - cfg := t.TempDir() - cmd := exec.Command(proxyBin, + cmd := testCommand(t, proxyBin, "--cluster-dir", clusterDir, "--port", strconv.Itoa(listenPort), "--ignore-persisted-port", ) - cmd.Env = append(os.Environ(), - "HOME="+cfg, "XDG_CONFIG_HOME="+cfg, "APPDATA="+cfg, "LOCALAPPDATA="+cfg, - ) stdin, err := cmd.StdinPipe() if err != nil { t.Fatalf("proxy stdin pipe: %v", err) diff --git a/services/tests/tls_test.go b/services/tests/tls_test.go index e0b7f0010..c43155ba8 100644 --- a/services/tests/tls_test.go +++ b/services/tests/tls_test.go @@ -17,7 +17,6 @@ import ( "net" "net/http" "os" - "os/exec" "path/filepath" "strings" "testing" @@ -125,7 +124,7 @@ func startNodeInfo(t *testing.T, instanceName string, extraArgs ...string) (clea t.Helper() // node-info no longer advertises over mDNS (discovery consolidation), so // these tests connect directly to the ports they pass. - cmd := exec.Command(nodeInfoBin, extraArgs...) + cmd := testCommand(t, nodeInfoBin, extraArgs...) cmd.Stderr = os.Stderr stdinPipe, err := cmd.StdinPipe() if err != nil { diff --git a/services/tests/workload_identity_interop_test.go b/services/tests/workload_identity_interop_test.go index f3879a74a..e6113cb24 100644 --- a/services/tests/workload_identity_interop_test.go +++ b/services/tests/workload_identity_interop_test.go @@ -158,8 +158,8 @@ func TestWorkloadCrossEngineIdentityDistinct(t *testing.T) { // hear the running workload AGAIN from the restarted manager — which only // happens if the broker rehydrated the fresh process's active set. func TestWorkloadManagerRehydratesActiveWorkloadOnRestart(t *testing.T) { - if portBusy(11435) { - t.Skip("ollama-proxy default port 11435 already in use; skipping") + if portBusy(11435) || portBusy(14320) { + t.Skip("ollama-proxy (11435) or workload-manager (14320) port already in use; skipping") } // Fake Ollama that accepts the request then blocks, keeping the proxied @@ -274,8 +274,8 @@ func TestWorkloadManagerRehydratesActiveWorkloadOnRestart(t *testing.T) { // state — otherwise a peer's wrongly-inferred "failed" (or a missed terminal) // can never be repaired after a restart. func TestWorkloadManagerRehydratesRecentTerminalOnRestart(t *testing.T) { - if portBusy(11435) { - t.Skip("ollama-proxy default port 11435 already in use; skipping") + if portBusy(11435) || portBusy(14320) { + t.Skip("ollama-proxy (11435) or workload-manager (14320) port already in use; skipping") } // Fake Ollama that completes immediately, so the workload reaches a terminal diff --git a/services/tests/workload_interop_test.go b/services/tests/workload_interop_test.go index 249f43350..b56e04024 100644 --- a/services/tests/workload_interop_test.go +++ b/services/tests/workload_interop_test.go @@ -29,7 +29,6 @@ import ( "net/http/httptest" "net/url" "os" - "os/exec" "strconv" "strings" "testing" @@ -80,8 +79,13 @@ func startBrokerProc(t *testing.T, args ...string) (io.WriteCloser, <-chan jsonr // cluster that directory describes (see newInterNodeCluster). func startBrokerProcInCluster(t *testing.T, clusterDir string, args ...string) (io.WriteCloser, <-chan jsonrpc.Message, func()) { t.Helper() + for _, arg := range args { + if arg == "--workload-manager-path" && portBusy(14320) { + t.Skip("workload-manager inter-node port 14320 already in use; skipping") + } + } args = append([]string{"--cluster-dir", clusterDir}, args...) - cmd := exec.Command(brokerBin, args...) + cmd := testCommand(t, brokerBin, args...) cmd.Stderr = os.Stderr stdinPipe, err := cmd.StdinPipe() From 6b58e5d7b646d96c78d53eb0590f85b294a968ab Mon Sep 17 00:00:00 2001 From: Jared Ping <13906915+JaredP94@users.noreply.github.com> Date: Tue, 3 Nov 2026 00:09:17 +0200 Subject: [PATCH 02/10] refactor(broker): make probe cancellation ownership explicit Go vet cannot prove the lifetime of the cancellation function assigned inside the watch loop. Extract probe launch into a helper that returns its context, cancellation function, and buffered result together; watch retains the same completion, deadline, and exit cleanup. Cover cancellation on stop, requested restart, and worker exit. Keep the existing unreleased broker 2.5.0 bump relative to v0.8.0, which already covers this internal refactor. Signed-off-by: Jared Ping <13906915+JaredP94@users.noreply.github.com> --- services/nvpair-ui-broker/supervisor.go | 16 +++++-- services/nvpair-ui-broker/supervisor_test.go | 49 ++++++++++++++++++++ 2 files changed, 61 insertions(+), 4 deletions(-) diff --git a/services/nvpair-ui-broker/supervisor.go b/services/nvpair-ui-broker/supervisor.go index a178068e1..b272f4172 100644 --- a/services/nvpair-ui-broker/supervisor.go +++ b/services/nvpair-ui-broker/supervisor.go @@ -79,6 +79,16 @@ func (p hangPolicy) withDefaults() hangPolicy { return p } +// startProbe transfers cancellation ownership to watch, which cancels on +// completion, deadline, or any exit. A buffered result lets a late probe finish +// without blocking after watch has moved on to a replacement handle. +func (p hangPolicy) startProbe() (context.Context, context.CancelFunc, <-chan error) { + ctx, cancel := context.WithTimeout(context.Background(), p.deadline) + result := make(chan error, 1) + go func() { result <- p.probe(ctx) }() + return ctx, cancel, result +} + // restartPolicy configures the hybrid auto-restart behaviour the broker // applies to a crashed worker. The defaults (see defaultRestartPolicy) // implement: exponential backoff between attempts, a bounded budget so a @@ -535,10 +545,8 @@ func (s *supervisor) watch(h supervisedHandle, onStable func(), onHealth func(er if probeC != nil { continue // Never overlap probes for the same live handle. } - probeCtx, cancel = context.WithTimeout(context.Background(), policy.deadline) - result := make(chan error, 1) - probeC, deadlineC = result, probeCtx.Done() - go func(ctx context.Context) { result <- policy.probe(ctx) }(probeCtx) + probeCtx, cancel, probeC = policy.startProbe() + deadlineC = probeCtx.Done() case err := <-probeC: if probeCtx.Err() != nil { err = probeCtx.Err() // A late success cannot beat the deadline. diff --git a/services/nvpair-ui-broker/supervisor_test.go b/services/nvpair-ui-broker/supervisor_test.go index 23aee3306..ce9dd7eff 100644 --- a/services/nvpair-ui-broker/supervisor_test.go +++ b/services/nvpair-ui-broker/supervisor_test.go @@ -32,6 +32,55 @@ func (h *fakeHandle) Stop() { h.stopOnce.Do(func() { close(h.stoppedC) }) } // crash closes the handle's done channel, simulating an unexpected exit. func (h *fakeHandle) crash() { close(h.done) } +func TestWatchCancelsInFlightProbeOnExit(t *testing.T) { + for _, exit := range []string{"stop", "restart", "worker-exit"} { + t.Run(exit, func(t *testing.T) { + sup := newSupervisor("cancel-test", restartPolicy{}, nil) + h := newFakeHandle() + started := make(chan context.Context, 1) + sup.hang = hangPolicy{ + interval: time.Millisecond, deadline: time.Hour, + probe: func(ctx context.Context) error { + started <- ctx + <-ctx.Done() + return ctx.Err() + }, + } + defer sup.stopOnce.Do(func() { close(sup.stopCh) }) + done := make(chan watchResult, 1) + go func() { done <- sup.watch(h, nil, nil) }() + var probeCtx context.Context + select { + case probeCtx = <-started: + case <-time.After(2 * time.Second): + t.Fatal("probe did not start") + } + want := watchStopped + switch exit { + case "stop": + sup.stopOnce.Do(func() { close(sup.stopCh) }) + case "restart": + sup.restartCh <- struct{}{} + want = watchRestart + case "worker-exit": + h.crash() + want = watchExited + } + select { + case got := <-done: + if got != want { + t.Fatalf("watch = %v, want %v", got, want) + } + case <-time.After(2 * time.Second): + t.Fatal("watch did not exit") + } + if !errors.Is(probeCtx.Err(), context.Canceled) { + t.Fatalf("in-flight probe not canceled on %s: %v", exit, probeCtx.Err()) + } + }) + } +} + func TestRestartPolicyBackoff(t *testing.T) { p := restartPolicy{baseDelay: time.Second, maxDelay: 16 * time.Second} cases := []struct { From e00b42a0c22e3f14fb76afdf99941077878101be Mon Sep 17 00:00:00 2001 From: Jared Ping <13906915+JaredP94@users.noreply.github.com> Date: Tue, 3 Nov 2026 00:09:17 +0200 Subject: [PATCH 03/10] ci: gate fork changes on desktop and Go checks Adapt NVIDIA/Personal-AI-Router@b4a0cc5 for this fork without release automation. Run component formatting, vet, and tests on macOS arm64 and Ubuntu; run the isolated cross-process suite on macOS only. Check the desktop contracts, attribution, types, lint, unit tests, dead code, and repository SPDX headers. Pin action tags and toolchains, cache npm and Go modules, use read-only checkout permissions, and normalize three existing test files so the formatting gate starts clean. Co-authored-by: Noah Tervalon Signed-off-by: Jared Ping <13906915+JaredP94@users.noreply.github.com> --- .github/workflows/ci.yml | 121 ++++++++++++++++++ services/nvpair-node-info/sampling_test.go | 16 +-- .../nvpair-ui-broker/nodeinfo_demand_test.go | 10 +- .../shared/splitlisten/splitlisten_test.go | 10 +- 4 files changed, 139 insertions(+), 18 deletions(-) create mode 100644 .github/workflows/ci.yml diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 000000000..64e646ced --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,121 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +# Adapted from NVIDIA/Personal-AI-Router@b4a0cc5. PR code runs only on +# disposable GitHub-hosted runners, without secrets or release automation. +name: CI + +on: + pull_request: + push: + branches: [main] + +permissions: + contents: read + +concurrency: + group: ci-${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: ${{ github.event_name == 'pull_request' }} + +env: + GOTOOLCHAIN: local + +jobs: + headers: + name: SPDX headers + runs-on: ubuntu-latest + timeout-minutes: 5 + steps: + - uses: actions/checkout@v5.0.0 + with: + persist-credentials: false + - uses: actions/setup-node@v5.0.0 + with: + node-version: '25.5.0' + - run: node scripts/spdx-headers.mjs + + desktop: + name: Desktop checks + runs-on: ubuntu-latest + timeout-minutes: 15 + defaults: + run: + working-directory: desktop + steps: + - uses: actions/checkout@v5.0.0 + with: + persist-credentials: false + - uses: actions/setup-node@v5.0.0 + with: + node-version: '25.5.0' + cache: npm + cache-dependency-path: desktop/package-lock.json + - run: npm ci --prefer-offline + - run: npm run typecheck + - run: npm run lint -- --max-warnings=0 + - run: npm run test:unit + - run: npm run dead-code:check + - run: npm run service-contracts:check + - run: npm run licenses:check + + services: + name: Go modules (${{ matrix.os }}) + # macos-latest is the standard arm64 hosted runner, not macos-*-intel. + runs-on: ${{ matrix.os }} + timeout-minutes: 20 + strategy: + fail-fast: false + matrix: + os: [macos-latest, ubuntu-latest] + steps: + - uses: actions/checkout@v5.0.0 + with: + persist-credentials: false + - uses: actions/setup-go@v6.0.0 + with: + go-version: '1.26.7' + cache-dependency-path: services/**/go.sum + - name: Linux test tools + if: runner.os == 'Linux' + run: sudo apt-get update -qq && sudo apt-get install -y -qq lsof iproute2 + - name: Check Go formatting + shell: bash + run: | + unformatted=$(gofmt -l services) + if [ -n "$unformatted" ]; then + printf '%s\n' "$unformatted" + exit 1 + fi + - name: Vet and test every service module (including libraries) + shell: bash + run: | + failed=0 + for mod in services/*/go.mod; do + dir=$(dirname "$mod") + # Cross-process tests run only on macOS in their own job. + if [ "$dir" = services/tests ]; then continue; fi + echo "==> $dir" + (cd "$dir" && go vet ./... && go test -count=1 -timeout=5m ./...) || failed=1 + done + exit "$failed" + + cross-process: + name: Cross-process tests (macOS arm64) + runs-on: macos-latest + timeout-minutes: 20 + steps: + - uses: actions/checkout@v5.0.0 + with: + persist-credentials: false + - uses: actions/setup-go@v6.0.0 + with: + go-version: '1.26.7' + cache-dependency-path: services/**/go.sum + # Run the source guard before executing any application binary. + # All runtime helpers must use per-test disposable app-data roots. + - name: Check process isolation + working-directory: services/tests + run: go test -count=1 -run '^Test(ProcessHelpersRequireIsolation|CommandEnvironmentIsPerTest)$' . + - name: Vet and run isolated cross-process suite + working-directory: services/tests + run: go vet ./... && go test -count=1 -timeout=15m ./... diff --git a/services/nvpair-node-info/sampling_test.go b/services/nvpair-node-info/sampling_test.go index 2c4738453..dac53231c 100644 --- a/services/nvpair-node-info/sampling_test.go +++ b/services/nvpair-node-info/sampling_test.go @@ -15,11 +15,11 @@ func TestPolicyTable(t *testing.T) { epoch := time.Unix(1000000, 0) cases := []struct { - name string - setup func(p *SamplingPolicy, now time.Time) - evalAt time.Duration - wantInterval time.Duration - wantMode SamplingMode + name string + setup func(p *SamplingPolicy, now time.Time) + evalAt time.Duration + wantInterval time.Duration + wantMode SamplingMode }{ { name: "clean idle: no leases, no demand", @@ -189,9 +189,9 @@ func TestPollIntentClassification(t *testing.T) { {"foreground", PollIntentForeground}, {"scheduling", PollIntentScheduling}, {"background", PollIntentBackground}, - {"", PollIntentForeground}, // missing header behaves as foreground - {"unknown", PollIntentForeground}, // unknown header behaves as foreground - {"FOREGROUND", PollIntentForeground},// unknown/cased behaves as foreground + {"", PollIntentForeground}, // missing header behaves as foreground + {"unknown", PollIntentForeground}, // unknown header behaves as foreground + {"FOREGROUND", PollIntentForeground}, // unknown/cased behaves as foreground } for _, tc := range cases { if got := parsePollIntent(tc.header); got != tc.want { diff --git a/services/nvpair-ui-broker/nodeinfo_demand_test.go b/services/nvpair-ui-broker/nodeinfo_demand_test.go index 62457a631..fe1f8a664 100644 --- a/services/nvpair-ui-broker/nodeinfo_demand_test.go +++ b/services/nvpair-ui-broker/nodeinfo_demand_test.go @@ -36,9 +36,9 @@ func TestWriteDemandFrame(t *testing.T) { } var frame struct { - JSONRPC string `json:"jsonrpc"` - Method string `json:"method"` - ID json.RawMessage `json:"id"` + JSONRPC string `json:"jsonrpc"` + Method string `json:"method"` + ID json.RawMessage `json:"id"` Params noderec.DemandParams `json:"params"` } if err := json.Unmarshal([]byte(line), &frame); err != nil { @@ -78,7 +78,7 @@ func TestBrokerDemandOnWorkloadTransitions(t *testing.T) { continue } var f struct { - Method string `json:"method"` + Method string `json:"method"` Params noderec.DemandParams `json:"params"` } if err := json.Unmarshal([]byte(l), &f); err == nil && f.Method == "stats/set-demand" { @@ -205,7 +205,7 @@ func TestBrokerDemandOnNodeInfoRestart(t *testing.T) { t.Fatalf("expected 1 frame on restart, got %d: %q", len(lines), buf.String()) } var f struct { - Method string `json:"method"` + Method string `json:"method"` Params noderec.DemandParams `json:"params"` } if err := json.Unmarshal([]byte(lines[0]), &f); err != nil { diff --git a/services/shared/splitlisten/splitlisten_test.go b/services/shared/splitlisten/splitlisten_test.go index 2843d3f1e..f8cd78fcc 100644 --- a/services/shared/splitlisten/splitlisten_test.go +++ b/services/shared/splitlisten/splitlisten_test.go @@ -247,11 +247,11 @@ func testServerTLSConfig(t *testing.T) *tls.Config { // scriptedListener is a net.Listener whose Accept behavior is driven by acceptFn. type scriptedListener struct { - addr net.Addr - acceptFn func(call int) (net.Conn, error) - mu sync.Mutex - calls int - closed chan struct{} + addr net.Addr + acceptFn func(call int) (net.Conn, error) + mu sync.Mutex + calls int + closed chan struct{} closeOnce sync.Once } From 08a89371fddc3c50951919a01a7ddce2b1a96db9 Mon Sep 17 00:00:00 2001 From: Jared Ping <13906915+JaredP94@users.noreply.github.com> Date: Tue, 3 Nov 2026 00:58:49 +0200 Subject: [PATCH 04/10] fix(engine-manager): distinguish reused bundle inodes Bind install claims to device, inode, and a generation timestamp: birth time on Darwin and ctime on Linux. A replaced Linux bundle must not inherit a claim when its inode is reused. Keep unchanged identities stable and preserve Darwin staging renames. Add deterministic reused-inode regressions including timestamp nanoseconds, and document conservative invalidation of prior claims. The existing unreleased engine-manager 1.4.0 bump over v0.8.0 already covers this fix. Signed-off-by: Jared Ping <13906915+JaredP94@users.noreply.github.com> --- services/nvpair-engine-manager/README.md | 6 +++- .../nvpair-engine-manager/fileid_darwin.go | 11 +++++++ .../fileid_darwin_test.go | 31 +++++++++++++++++++ .../nvpair-engine-manager/fileid_linux.go | 12 +++++++ .../fileid_linux_test.go | 27 ++++++++++++++++ services/nvpair-engine-manager/fileid_unix.go | 14 ++++++--- 6 files changed, 96 insertions(+), 5 deletions(-) create mode 100644 services/nvpair-engine-manager/fileid_darwin.go create mode 100644 services/nvpair-engine-manager/fileid_darwin_test.go create mode 100644 services/nvpair-engine-manager/fileid_linux.go create mode 100644 services/nvpair-engine-manager/fileid_linux_test.go diff --git a/services/nvpair-engine-manager/README.md b/services/nvpair-engine-manager/README.md index fab6671e8..6b380a5d1 100644 --- a/services/nvpair-engine-manager/README.md +++ b/services/nvpair-engine-manager/README.md @@ -156,7 +156,11 @@ version only decides what a fresh install starts from. `install.creates` names `/Applications/oMLX.app`: an install that finds an app already there opens it but does not record it as PAIR's (and drops any earlier claim), so it is never offered for uninstall. A claim records the created app's -file identity and covers that exact bundle only. A copy put there later is a +file identity and covers that exact bundle only. On macOS this includes birth +time (stable across the install's rename); on Linux it includes ctime so a reused +inode cannot inherit the claim. Linux metadata changes also invalidate the claim. +Older device/inode-only claims no longer authorize removal; reinstall through +PAIR to establish a new claim. A copy put there later is a different file, and so is a bundle oMLX's own updater swaps in: after an in-app update PAIR no longer offers uninstall, and the app is removed from the Finder. Uninstalling an oMLX PAIR installed quits the app and removes `/Applications/oMLX.app` and the two shims it writes (`omlx`, diff --git a/services/nvpair-engine-manager/fileid_darwin.go b/services/nvpair-engine-manager/fileid_darwin.go new file mode 100644 index 000000000..0b69c23fd --- /dev/null +++ b/services/nvpair-engine-manager/fileid_darwin.go @@ -0,0 +1,11 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +package main + +import "syscall" + +// Birth time survives the staging-to-destination rename and metadata changes. +func fileGenerationTime(st *syscall.Stat_t) syscall.Timespec { + return st.Birthtimespec +} diff --git a/services/nvpair-engine-manager/fileid_darwin_test.go b/services/nvpair-engine-manager/fileid_darwin_test.go new file mode 100644 index 000000000..42c216544 --- /dev/null +++ b/services/nvpair-engine-manager/fileid_darwin_test.go @@ -0,0 +1,31 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "syscall" + "testing" +) + +// Reusing a device/inode must not transfer an install claim to a later bundle. +func TestFileIdentityDistinguishesReusedInode(t *testing.T) { + st := syscall.Stat_t{Dev: 1, Ino: 2, Birthtimespec: syscall.Timespec{Sec: 3, Nsec: 4}} + original := fileIdentityFromStat(&st) + if got := fileIdentityFromStat(&st); got != original { + t.Fatal("unchanged bundle identity is unstable") + } + st.Ctimespec = syscall.Timespec{Sec: 5, Nsec: 6} + if got := fileIdentityFromStat(&st); got != original { + t.Fatal("metadata changes must not invalidate a Darwin bundle's identity") + } + st.Birthtimespec.Nsec++ + if got := fileIdentityFromStat(&st); got == original { + t.Fatal("reused inode with a different birth nanosecond retained the old identity") + } + st.Birthtimespec.Nsec-- + st.Birthtimespec.Sec++ + if got := fileIdentityFromStat(&st); got == original { + t.Fatal("reused inode with a different birth second retained the old identity") + } +} diff --git a/services/nvpair-engine-manager/fileid_linux.go b/services/nvpair-engine-manager/fileid_linux.go new file mode 100644 index 000000000..1c0edc76a --- /dev/null +++ b/services/nvpair-engine-manager/fileid_linux.go @@ -0,0 +1,12 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +package main + +import "syscall" + +// Stat_t has no birth time on Linux. ctime distinguishes reused inodes and +// stays stable while the installed bundle is unchanged. +func fileGenerationTime(st *syscall.Stat_t) syscall.Timespec { + return st.Ctim +} diff --git a/services/nvpair-engine-manager/fileid_linux_test.go b/services/nvpair-engine-manager/fileid_linux_test.go new file mode 100644 index 000000000..45caecc87 --- /dev/null +++ b/services/nvpair-engine-manager/fileid_linux_test.go @@ -0,0 +1,27 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "syscall" + "testing" +) + +// Linux can immediately reuse the inode of a deleted bundle directory. +func TestFileIdentityDistinguishesReusedInode(t *testing.T) { + st := syscall.Stat_t{Dev: 1, Ino: 2, Ctim: syscall.Timespec{Sec: 3, Nsec: 4}} + original := fileIdentityFromStat(&st) + if got := fileIdentityFromStat(&st); got != original { + t.Fatal("unchanged bundle identity is unstable") + } + st.Ctim.Nsec++ + if got := fileIdentityFromStat(&st); got == original { + t.Fatal("reused inode with a different change nanosecond retained the old identity") + } + st.Ctim.Nsec-- + st.Ctim.Sec++ + if got := fileIdentityFromStat(&st); got == original { + t.Fatal("reused inode with a different change second retained the old identity") + } +} diff --git a/services/nvpair-engine-manager/fileid_unix.go b/services/nvpair-engine-manager/fileid_unix.go index b71b98040..256403e5b 100644 --- a/services/nvpair-engine-manager/fileid_unix.go +++ b/services/nvpair-engine-manager/fileid_unix.go @@ -1,7 +1,7 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -//go:build !windows +//go:build darwin || linux package main @@ -11,8 +11,9 @@ import ( "syscall" ) -// fileIdentity is a path's device and inode, without following a final link, -// or "" when it cannot be read. +// fileIdentity combines device, inode, and a generation timestamp without +// following a final link. The timestamp prevents inode reuse from transferring +// an install claim to a replacement bundle. It returns "" on read failure. func fileIdentity(path string) string { info, err := os.Lstat(path) if err != nil { @@ -22,5 +23,10 @@ func fileIdentity(path string) string { if !ok { return "" } - return fmt.Sprintf("%d:%d", st.Dev, st.Ino) + return fileIdentityFromStat(st) +} + +func fileIdentityFromStat(st *syscall.Stat_t) string { + stamp := fileGenerationTime(st) + return fmt.Sprintf("%d:%d:%d:%d", st.Dev, st.Ino, stamp.Sec, stamp.Nsec) } From 7b73cddce13b80114b608bafb99fef8349d8e282 Mon Sep 17 00:00:00 2001 From: Jared Ping <13906915+JaredP94@users.noreply.github.com> Date: Tue, 3 Nov 2026 00:59:39 +0200 Subject: [PATCH 05/10] ci: warm offline license dependencies and pin macOS arm64 Install the pinned Go toolchain in the desktop job and download declared modules, including Windows-only dependencies, before the offline license check. Verify the preparation does not change service manifests. The first run never acquired either macos-latest runner: both jobs had zero executed steps and runner_id 0. Pin the supported macos-15 arm64 image instead of raising test timeouts for an infrastructure queue failure. Emit verbose Go test progress for future slow-test diagnosis. Signed-off-by: Jared Ping <13906915+JaredP94@users.noreply.github.com> --- .github/workflows/ci.yml | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 64e646ced..0e639e634 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -50,6 +50,21 @@ jobs: node-version: '25.5.0' cache: npm cache-dependency-path: desktop/package-lock.json + - uses: actions/setup-go@v6.0.0 + with: + go-version: '1.26.7' + cache-dependency-path: services/**/go.sum + # The license checker is deliberately offline and enumerates every + # release target, including Windows-only dependencies. A native + # build alone would leave those dependencies out of the cache. + - name: Populate Go license dependency cache + working-directory: . + shell: bash + run: | + for mod in services/*/go.mod; do + (cd "$(dirname "$mod")" && go mod download) + done + git diff --exit-code -- services - run: npm ci --prefer-offline - run: npm run typecheck - run: npm run lint -- --max-warnings=0 @@ -60,13 +75,14 @@ jobs: services: name: Go modules (${{ matrix.os }}) - # macos-latest is the standard arm64 hosted runner, not macos-*-intel. + # Pin the supported arm64 image: macos-latest failed runner acquisition + # during the macOS 26 rollout, before any test step could execute. runs-on: ${{ matrix.os }} timeout-minutes: 20 strategy: fail-fast: false matrix: - os: [macos-latest, ubuntu-latest] + os: [macos-15, ubuntu-latest] steps: - uses: actions/checkout@v5.0.0 with: @@ -95,13 +111,13 @@ jobs: # Cross-process tests run only on macOS in their own job. if [ "$dir" = services/tests ]; then continue; fi echo "==> $dir" - (cd "$dir" && go vet ./... && go test -count=1 -timeout=5m ./...) || failed=1 + (cd "$dir" && go vet ./... && go test -count=1 -timeout=5m -v ./...) || failed=1 done exit "$failed" cross-process: name: Cross-process tests (macOS arm64) - runs-on: macos-latest + runs-on: macos-15 timeout-minutes: 20 steps: - uses: actions/checkout@v5.0.0 @@ -118,4 +134,4 @@ jobs: run: go test -count=1 -run '^Test(ProcessHelpersRequireIsolation|CommandEnvironmentIsPerTest)$' . - name: Vet and run isolated cross-process suite working-directory: services/tests - run: go vet ./... && go test -count=1 -timeout=15m ./... + run: go vet ./... && go test -count=1 -timeout=15m -v ./... From 2b5613cb5c2cf360ed91729b7c24cb127d0d3a60 Mon Sep 17 00:00:00 2001 From: Jared Ping <13906915+JaredP94@users.noreply.github.com> Date: Tue, 3 Nov 2026 01:17:14 +0200 Subject: [PATCH 06/10] ci: configure on-link multicast routing for macOS discovery The first acquired arm64 runner completed all tests in eight minutes, but real mDNS sends failed with no route to host on en0. Add an explicit on-link IPv4 multicast route on the disposable runner before running the complete isolated cross-process suite. Keep every discovery and workload assertion enabled. Signed-off-by: Jared Ping <13906915+JaredP94@users.noreply.github.com> --- .github/workflows/ci.yml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0e639e634..b7e92c44c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -127,6 +127,13 @@ jobs: with: go-version: '1.26.7' cache-dependency-path: services/**/go.sum + # Hosted macOS VMs need an explicit on-link multicast route for + # real mDNS discovery; otherwise sends fail with no route to host. + - name: Configure runner multicast route + run: | + interface=$(route -n get default | awk '/interface:/{print $2}') + sudo route -n add -net 224.0.0.0/4 -interface "$interface" + route -n get 224.0.0.251 # Run the source guard before executing any application binary. # All runtime helpers must use per-test disposable app-data roots. - name: Check process isolation From 6d8d3db57c917f36d1f1ee911790f1b838aa3912 Mon Sep 17 00:00:00 2001 From: Jared Ping <13906915+JaredP94@users.noreply.github.com> Date: Tue, 3 Nov 2026 06:27:50 +0200 Subject: [PATCH 07/10] ci(tests): retain cross-process timeout diagnostics Stream verbose Go JSON events to both the job log and an always-uploaded artifact. Keep the Go deadline below the job deadline so a stuck suite reports its active test and goroutines rather than losing all evidence to job cancellation. Signed-off-by: Jared Ping <13906915+JaredP94@users.noreply.github.com> --- .github/workflows/ci.yml | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b7e92c44c..2dc3742ac 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -139,6 +139,23 @@ jobs: - name: Check process isolation working-directory: services/tests run: go test -count=1 -run '^Test(ProcessHelpersRequireIsolation|CommandEnvironmentIsPerTest)$' . - - name: Vet and run isolated cross-process suite + - name: Vet cross-process suite working-directory: services/tests - run: go vet ./... && go test -count=1 -timeout=15m -v ./... + run: go vet ./... + # Leave five minutes for setup and artifact upload. The Go deadline + # prints the active test and goroutines before the job is killed. + - name: Run isolated cross-process suite with diagnostics + working-directory: services/tests + shell: bash + run: | + set -o pipefail + go test -v -count=1 -timeout=15m -json ./... 2>&1 \ + | tee "$RUNNER_TEMP/cross-process.jsonl" \ + | jq --unbuffered -j 'select(.Output != null) | .Output' + - name: Upload cross-process diagnostics + if: always() + uses: actions/upload-artifact@v4.6.2 + with: + name: cross-process-macos-arm64 + path: ${{ runner.temp }}/cross-process.jsonl + if-no-files-found: warn From 2892745979cdd127ba1c529f0f68f73441f99c62 Mon Sep 17 00:00:00 2001 From: Jared Ping <13906915+JaredP94@users.noreply.github.com> Date: Tue, 3 Nov 2026 07:26:13 +0200 Subject: [PATCH 08/10] ci(tests): run cross-process tests on Linux too, and name the macOS multicast gap The macOS cross-process job failed 12 discovery tests on every run, and not because of a hang: with the diagnostics in place the suite finished in eight minutes. Hosted macOS runners cannot send IPv4 multicast at all; every mDNS send to 224.0.0.251 fails with "no route to host", even with an explicit 224.0.0.0/4 route, so no node is ever discovered. Run the cross-process suite on ubuntu-latest as well, where multicast works, so discovery is exercised in CI. Tests that need real mDNS now probe whether this host can multicast and skip only when it cannot, naming the environment; on any machine that can, including a developer Mac, they run as before. Drop the route step, which did not help. Signed-off-by: Jared Ping <13906915+JaredP94@users.noreply.github.com> --- .github/workflows/ci.yml | 24 +++++---- services/tests/broker_test.go | 1 + services/tests/discovery_relay_test.go | 1 + services/tests/discovery_test.go | 3 ++ services/tests/ghost_node_test.go | 1 + services/tests/models_http_test.go | 1 + services/tests/models_refresh_test.go | 1 + services/tests/multicast_probe_test.go | 54 +++++++++++++++++++ services/tests/secure_inference_test.go | 1 + .../tests/workload_identity_interop_test.go | 2 + services/tests/workload_interop_test.go | 2 + 11 files changed, 81 insertions(+), 10 deletions(-) create mode 100644 services/tests/multicast_probe_test.go diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2dc3742ac..a09a632b0 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -116,9 +116,17 @@ jobs: exit "$failed" cross-process: - name: Cross-process tests (macOS arm64) - runs-on: macos-15 + name: Cross-process tests (${{ matrix.os }}) + runs-on: ${{ matrix.os }} timeout-minutes: 20 + strategy: + fail-fast: false + # Linux runners can send mDNS multicast, so discovery is exercised + # there. Hosted macOS VMs cannot (every send to 224.0.0.251 fails + # with "no route to host"); on macOS the discovery tests skip with + # that reason named, and everything else runs. + matrix: + os: [ubuntu-latest, macos-15] steps: - uses: actions/checkout@v5.0.0 with: @@ -127,13 +135,9 @@ jobs: with: go-version: '1.26.7' cache-dependency-path: services/**/go.sum - # Hosted macOS VMs need an explicit on-link multicast route for - # real mDNS discovery; otherwise sends fail with no route to host. - - name: Configure runner multicast route - run: | - interface=$(route -n get default | awk '/interface:/{print $2}') - sudo route -n add -net 224.0.0.0/4 -interface "$interface" - route -n get 224.0.0.251 + - name: Install port-owner tools + if: runner.os == 'Linux' + run: sudo apt-get update -qq && sudo apt-get install -y -qq lsof iproute2 # Run the source guard before executing any application binary. # All runtime helpers must use per-test disposable app-data roots. - name: Check process isolation @@ -156,6 +160,6 @@ jobs: if: always() uses: actions/upload-artifact@v4.6.2 with: - name: cross-process-macos-arm64 + name: cross-process-${{ matrix.os }} path: ${{ runner.temp }}/cross-process.jsonl if-no-files-found: warn diff --git a/services/tests/broker_test.go b/services/tests/broker_test.go index 235a5b565..c947be6fd 100644 --- a/services/tests/broker_test.go +++ b/services/tests/broker_test.go @@ -191,6 +191,7 @@ func waitForPushContaining(t *testing.T, msgs <-chan jsonrpc.Message, instance s } func TestCrossProcessBrokerSubscription(t *testing.T) { + requireMulticast(t) const ( inst1 = "xproc-broker-1" inst2 = "xproc-broker-2" diff --git a/services/tests/discovery_relay_test.go b/services/tests/discovery_relay_test.go index 7ac1510a9..fd1ed52f6 100644 --- a/services/tests/discovery_relay_test.go +++ b/services/tests/discovery_relay_test.go @@ -17,6 +17,7 @@ import ( // ni=14318. This is the new consolidated discovery record (distinct from // node-info's own transitional _nvpair-node-info advertisement). func TestBrokerAdvertisesNodeRecord(t *testing.T) { + requireMulticast(t) _, msgs, stderr, cleanup := startBrokerWith(t, "--node-info-path", nodeInfoBin) t.Cleanup(cleanup) // Drain the broker's stdout/stderr so it doesn't stall on a full pipe while diff --git a/services/tests/discovery_test.go b/services/tests/discovery_test.go index cd9a6c05f..59da2fc18 100644 --- a/services/tests/discovery_test.go +++ b/services/tests/discovery_test.go @@ -15,6 +15,7 @@ import ( const testDomain = "local." func TestInProcessDiscovery(t *testing.T) { + requireMulticast(t) const ( service = "_test-disc._tcp" instance = "in-proc-test" @@ -71,6 +72,7 @@ func TestInProcessDiscovery(t *testing.T) { } func TestInProcessMultipleInstances(t *testing.T) { + requireMulticast(t) const service = "_test-multi._tcp" instances := []string{"node-alpha", "node-beta", "node-gamma"} @@ -121,6 +123,7 @@ func TestInProcessMultipleInstances(t *testing.T) { } func TestInProcessRemoval(t *testing.T) { + requireMulticast(t) const ( service = "_test-rmv._tcp" instance = "removable-node" diff --git a/services/tests/ghost_node_test.go b/services/tests/ghost_node_test.go index 31042c890..a50bae8c1 100644 --- a/services/tests/ghost_node_test.go +++ b/services/tests/ghost_node_test.go @@ -41,6 +41,7 @@ func TestScannerEvictsRecordSupersededAtItsAddress(t *testing.T) { if testing.Short() { t.Skip("multicast + miss-threshold timing; skipped under -short") } + requireMulticast(t) const ( peerInstance = "ghost-peer-host" diff --git a/services/tests/models_http_test.go b/services/tests/models_http_test.go index baf5601de..20e3708e3 100644 --- a/services/tests/models_http_test.go +++ b/services/tests/models_http_test.go @@ -24,6 +24,7 @@ import ( ) func TestModelsHTTPEnrichment(t *testing.T) { + requireMulticast(t) // Stand-in engine-manager HTTP surface serving the model list. srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if r.URL.Path != "/v1/models" { diff --git a/services/tests/models_refresh_test.go b/services/tests/models_refresh_test.go index 608787983..9bd318cf3 100644 --- a/services/tests/models_refresh_test.go +++ b/services/tests/models_refresh_test.go @@ -55,6 +55,7 @@ func (s *swapModels) handler() http.HandlerFunc { } func TestModelsPeriodicRefreshConvergesWithoutMDNSChange(t *testing.T) { + requireMulticast(t) // Start with an empty inventory (engine present but serving no models yet). stub := &swapModels{} stub.set(map[string]any{"models": []string{}}) diff --git a/services/tests/multicast_probe_test.go b/services/tests/multicast_probe_test.go new file mode 100644 index 000000000..3c039e6db --- /dev/null +++ b/services/tests/multicast_probe_test.go @@ -0,0 +1,54 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +package tests + +import ( + "encoding/binary" + "net" + "sync" + "testing" + "time" +) + +var ( + multicastOnce sync.Once + multicastErr error +) + +// requireMulticast skips a test that needs real mDNS discovery when this host +// cannot send IPv4 multicast at all. GitHub's hosted macOS runners are such a +// host: every send to 224.0.0.251 fails with "no route to host", so no node is +// ever discovered. On any machine that can multicast, the test runs as usual; +// the skip names the environment so it is never mistaken for a pass. +func requireMulticast(t *testing.T) { + t.Helper() + multicastOnce.Do(func() { multicastErr = probeMulticast() }) + if multicastErr != nil { + t.Skipf("environment: this host cannot send mDNS multicast (%v); this test needs real mDNS discovery", multicastErr) + } +} + +// probeMulticast sends one mDNS PTR query for a name nothing answers, the same +// kind of packet discovery sends, and reports whether it left the host. +func probeMulticast() error { + conn, err := net.DialUDP("udp4", nil, &net.UDPAddr{IP: net.IPv4(224, 0, 0, 251), Port: 5353}) + if err != nil { + return err + } + defer conn.Close() + _ = conn.SetWriteDeadline(time.Now().Add(2 * time.Second)) + _, err = conn.Write(mdnsProbeQuery()) + return err +} + +func mdnsProbeQuery() []byte { + msg := make([]byte, 12) + binary.BigEndian.PutUint16(msg[4:], 1) // one question + for _, label := range []string{"_nvpair-ci-probe", "_tcp", "local"} { + msg = append(msg, byte(len(label))) + msg = append(msg, label...) + } + msg = append(msg, 0, 0, 12, 0, 1) // root, type PTR, class IN + return msg +} diff --git a/services/tests/secure_inference_test.go b/services/tests/secure_inference_test.go index 7a3aef0f8..fc9ee4495 100644 --- a/services/tests/secure_inference_test.go +++ b/services/tests/secure_inference_test.go @@ -237,6 +237,7 @@ func startFakeOllama(t *testing.T) (host string, port int, generates *int32) { } func TestSecureInferenceClusterMTLS(t *testing.T) { + requireMulticast(t) baseA, baseB, baseC := t.TempDir(), t.TempDir(), t.TempDir() clusterA := filepath.Join(baseA, "cluster") clusterB := filepath.Join(baseB, "cluster") diff --git a/services/tests/workload_identity_interop_test.go b/services/tests/workload_identity_interop_test.go index e6113cb24..d6afd0c6b 100644 --- a/services/tests/workload_identity_interop_test.go +++ b/services/tests/workload_identity_interop_test.go @@ -158,6 +158,7 @@ func TestWorkloadCrossEngineIdentityDistinct(t *testing.T) { // hear the running workload AGAIN from the restarted manager — which only // happens if the broker rehydrated the fresh process's active set. func TestWorkloadManagerRehydratesActiveWorkloadOnRestart(t *testing.T) { + requireMulticast(t) if portBusy(11435) || portBusy(14320) { t.Skip("ollama-proxy (11435) or workload-manager (14320) port already in use; skipping") } @@ -274,6 +275,7 @@ func TestWorkloadManagerRehydratesActiveWorkloadOnRestart(t *testing.T) { // state — otherwise a peer's wrongly-inferred "failed" (or a missed terminal) // can never be repaired after a restart. func TestWorkloadManagerRehydratesRecentTerminalOnRestart(t *testing.T) { + requireMulticast(t) if portBusy(11435) || portBusy(14320) { t.Skip("ollama-proxy (11435) or workload-manager (14320) port already in use; skipping") } diff --git a/services/tests/workload_interop_test.go b/services/tests/workload_interop_test.go index b56e04024..bfbbc978e 100644 --- a/services/tests/workload_interop_test.go +++ b/services/tests/workload_interop_test.go @@ -282,6 +282,7 @@ func TestWorkloadOutOfOrderSuppressed(t *testing.T) { // evicts it (miss threshold + liveness probe), and the broker synthesizes the // terminal workloads:upsert(state=failed). func TestWorkloadFailedOnNodeLoss(t *testing.T) { + requireMulticast(t) // The peer's mDNS instance name and its stable HostUUID differ on purpose: // workloads are stamped/keyed by HostUUID, and the node-loss sweep // must match on that UUID, not the display name. Keying by name would leave @@ -369,6 +370,7 @@ func TestWorkloadFailedOnNodeLoss(t *testing.T) { // workload event that the broker stamps and forwards to the manager, which // broadcasts it to a discovered peer. func TestWorkloadManagerOutboundBroadcast(t *testing.T) { + requireMulticast(t) // Fake Ollama upstream the proxy forwards to (200 on any request). ollama := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { io.Copy(io.Discard, r.Body) From f085dfc84b1fc9f425845ec5119c0faec05f0927 Mon Sep 17 00:00:00 2001 From: Jared Ping <13906915+JaredP94@users.noreply.github.com> Date: Tue, 3 Nov 2026 07:40:02 +0200 Subject: [PATCH 09/10] test(services): probe multicast per interface, as discovery sends Review follow-up. The multicast probe tried only the default route, but discovery sends through every eligible interface, so a multicast-capable LAN without a default route would have skipped the discovery tests. The probe now sends out of each up, multicast-capable IPv4 interface, runs the tests if any succeeds, skips only when every one fails for lack of a multicast route, and fails the test on any other socket error. The decision has its own table test. Also corrects a stale workflow comment. Signed-off-by: Jared Ping <13906915+JaredP94@users.noreply.github.com> --- .github/workflows/ci.yml | 2 +- services/tests/go.mod | 2 +- services/tests/multicast_probe_test.go | 138 ++++++++++++++++++++++--- 3 files changed, 126 insertions(+), 16 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a09a632b0..4b752ffa8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -108,7 +108,7 @@ jobs: failed=0 for mod in services/*/go.mod; do dir=$(dirname "$mod") - # Cross-process tests run only on macOS in their own job. + # Cross-process tests run in their own job (Linux and macOS). if [ "$dir" = services/tests ]; then continue; fi echo "==> $dir" (cd "$dir" && go vet ./... && go test -count=1 -timeout=5m -v ./...) || failed=1 diff --git a/services/tests/go.mod b/services/tests/go.mod index 38bfb3a9c..f61403558 100644 --- a/services/tests/go.mod +++ b/services/tests/go.mod @@ -4,6 +4,7 @@ go 1.25.0 require ( github.com/grandcat/zeroconf v1.0.0 + golang.org/x/net v0.58.0 nvpair-shared v0.0.0-00010101000000-000000000000 ) @@ -14,7 +15,6 @@ require ( github.com/google/uuid v1.6.0 // indirect github.com/miekg/dns v1.1.55 // indirect golang.org/x/mod v0.38.0 // indirect - golang.org/x/net v0.58.0 // indirect golang.org/x/sync v0.22.0 // indirect golang.org/x/sys v0.47.0 // indirect golang.org/x/tools v0.48.0 // indirect diff --git a/services/tests/multicast_probe_test.go b/services/tests/multicast_probe_test.go index 3c039e6db..feafc76ef 100644 --- a/services/tests/multicast_probe_test.go +++ b/services/tests/multicast_probe_test.go @@ -5,43 +5,131 @@ package tests import ( "encoding/binary" + "errors" + "fmt" "net" + "strings" "sync" + "syscall" "testing" "time" + + "golang.org/x/net/ipv4" ) var ( - multicastOnce sync.Once - multicastErr error + multicastOnce sync.Once + multicastSkip string + multicastFailed error ) // requireMulticast skips a test that needs real mDNS discovery when this host -// cannot send IPv4 multicast at all. GitHub's hosted macOS runners are such a -// host: every send to 224.0.0.251 fails with "no route to host", so no node is -// ever discovered. On any machine that can multicast, the test runs as usual; -// the skip names the environment so it is never mistaken for a pass. +// cannot send IPv4 multicast through any interface. GitHub's hosted macOS +// runners are such a host: every send to 224.0.0.251 fails with "no route to +// host", so no node is ever discovered. Like discovery, the probe tries every +// eligible interface, so a host that can multicast on any of them runs the +// test. An unexpected socket error fails the test rather than skipping it. func requireMulticast(t *testing.T) { t.Helper() - multicastOnce.Do(func() { multicastErr = probeMulticast() }) - if multicastErr != nil { - t.Skipf("environment: this host cannot send mDNS multicast (%v); this test needs real mDNS discovery", multicastErr) + multicastOnce.Do(func() { multicastSkip, multicastFailed = decideMulticast(probeInterfaces()) }) + if multicastFailed != nil { + t.Fatalf("multicast probe: %v", multicastFailed) + } + if multicastSkip != "" { + t.Skipf("environment: this host cannot send mDNS multicast (%s); this test needs real mDNS discovery", multicastSkip) + } +} + +// interfaceProbe is one eligible interface's send result. +type interfaceProbe struct { + name string + err error +} + +// probeInterfaces sends one mDNS PTR query, for a name nothing answers, out of +// every up, multicast-capable, non-loopback interface with an IPv4 address — +// the interfaces discovery sends on. +func probeInterfaces() []interfaceProbe { + ifaces, err := net.Interfaces() + if err != nil { + return []interfaceProbe{{name: "interfaces", err: err}} + } + var out []interfaceProbe + for i := range ifaces { + ifi := ifaces[i] + if ifi.Flags&net.FlagUp == 0 || ifi.Flags&net.FlagMulticast == 0 || ifi.Flags&net.FlagLoopback != 0 { + continue + } + src := firstIPv4(&ifi) + if src == nil { + continue + } + out = append(out, interfaceProbe{name: ifi.Name, err: sendProbe(&ifi, src)}) + } + return out +} + +func firstIPv4(ifi *net.Interface) net.IP { + addrs, err := ifi.Addrs() + if err != nil { + return nil } + for _, a := range addrs { + if ipnet, ok := a.(*net.IPNet); ok { + if ip4 := ipnet.IP.To4(); ip4 != nil { + return ip4 + } + } + } + return nil } -// probeMulticast sends one mDNS PTR query for a name nothing answers, the same -// kind of packet discovery sends, and reports whether it left the host. -func probeMulticast() error { - conn, err := net.DialUDP("udp4", nil, &net.UDPAddr{IP: net.IPv4(224, 0, 0, 251), Port: 5353}) +func sendProbe(ifi *net.Interface, src net.IP) error { + conn, err := net.ListenUDP("udp4", &net.UDPAddr{IP: src}) if err != nil { return err } defer conn.Close() + pc := ipv4.NewPacketConn(conn) + if err := pc.SetMulticastInterface(ifi); err != nil { + return err + } + _ = pc.SetMulticastTTL(255) _ = conn.SetWriteDeadline(time.Now().Add(2 * time.Second)) - _, err = conn.Write(mdnsProbeQuery()) + _, err = conn.WriteToUDP(mdnsProbeQuery(), &net.UDPAddr{IP: net.IPv4(224, 0, 0, 251), Port: 5353}) return err } +// decideMulticast turns the probe results into a decision: run when any +// interface sent; skip, naming why, when none could because the host has no +// usable multicast route (or no eligible interface); fail on anything else. +func decideMulticast(probes []interfaceProbe) (skip string, failed error) { + if len(probes) == 0 { + return "no up, multicast-capable IPv4 interface", nil + } + var reasons []string + for _, p := range probes { + if p.err == nil { + return "", nil + } + if !noMulticastRoute(p.err) { + failed = errors.Join(failed, fmt.Errorf("%s: %w", p.name, p.err)) + } + reasons = append(reasons, fmt.Sprintf("%s: %v", p.name, p.err)) + } + if failed != nil { + return "", failed + } + return strings.Join(reasons, "; "), nil +} + +// noMulticastRoute reports whether a send failed because the host has no +// route for multicast out of that interface, the environment limitation the +// skip exists for. +func noMulticastRoute(err error) bool { + return errors.Is(err, syscall.EHOSTUNREACH) || errors.Is(err, syscall.ENETUNREACH) || errors.Is(err, syscall.EADDRNOTAVAIL) +} + func mdnsProbeQuery() []byte { msg := make([]byte, 12) binary.BigEndian.PutUint16(msg[4:], 1) // one question @@ -52,3 +140,25 @@ func mdnsProbeQuery() []byte { msg = append(msg, 0, 0, 12, 0, 1) // root, type PTR, class IN return msg } + +func TestDecideMulticast(t *testing.T) { + noRoute := &net.OpError{Op: "write", Net: "udp4", Err: syscall.EHOSTUNREACH} + odd := &net.OpError{Op: "write", Net: "udp4", Err: syscall.EPERM} + for _, tc := range []struct { + name string + probes []interfaceProbe + wantSkip bool + wantFail bool + }{ + {"no eligible interface", nil, true, false}, + {"one interface can send", []interfaceProbe{{"en0", nil}}, false, false}, + {"any interface sending wins", []interfaceProbe{{"en0", noRoute}, {"en1", nil}}, false, false}, + {"no route anywhere", []interfaceProbe{{"en0", noRoute}, {"utun3", noRoute}}, true, false}, + {"an unexpected error fails", []interfaceProbe{{"en0", noRoute}, {"en1", odd}}, false, true}, + } { + skip, failed := decideMulticast(tc.probes) + if (skip != "") != tc.wantSkip || (failed != nil) != tc.wantFail { + t.Errorf("%s: skip=%q failed=%v", tc.name, skip, failed) + } + } +} From cb6ce6322f485d6adba4c1ca6bd8cd2fbb06a730 Mon Sep 17 00:00:00 2001 From: Jared Ping <13906915+JaredP94@users.noreply.github.com> Date: Tue, 3 Nov 2026 07:41:19 +0200 Subject: [PATCH 10/10] test(services): report every unexpected multicast probe error Review follow-up. The probe stopped at the first interface that sent, so an unexpected error on another went unseen. Every result is now inspected: unexpected errors are logged even when another interface sent (the test still runs, as discovery tolerates per-interface failures), and fail the test when no interface could send. Signed-off-by: Jared Ping <13906915+JaredP94@users.noreply.github.com> --- services/tests/multicast_probe_test.go | 69 +++++++++++++++++--------- 1 file changed, 45 insertions(+), 24 deletions(-) diff --git a/services/tests/multicast_probe_test.go b/services/tests/multicast_probe_test.go index feafc76ef..05828e0dd 100644 --- a/services/tests/multicast_probe_test.go +++ b/services/tests/multicast_probe_test.go @@ -21,6 +21,7 @@ var ( multicastOnce sync.Once multicastSkip string multicastFailed error + multicastNotes []string ) // requireMulticast skips a test that needs real mDNS discovery when this host @@ -31,7 +32,12 @@ var ( // test. An unexpected socket error fails the test rather than skipping it. func requireMulticast(t *testing.T) { t.Helper() - multicastOnce.Do(func() { multicastSkip, multicastFailed = decideMulticast(probeInterfaces()) }) + multicastOnce.Do(func() { + multicastSkip, multicastFailed, multicastNotes = decideMulticast(probeInterfaces()) + }) + for _, note := range multicastNotes { + t.Logf("multicast probe: %s", note) + } if multicastFailed != nil { t.Fatalf("multicast probe: %v", multicastFailed) } @@ -100,27 +106,39 @@ func sendProbe(ifi *net.Interface, src net.IP) error { return err } -// decideMulticast turns the probe results into a decision: run when any -// interface sent; skip, naming why, when none could because the host has no -// usable multicast route (or no eligible interface); fail on anything else. -func decideMulticast(probes []interfaceProbe) (skip string, failed error) { +// decideMulticast turns the probe results into a decision. Every result is +// inspected: an unexpected error on any interface is reported as a note even +// when another interface sent, so nothing is masked. The test runs when any +// interface sent, since discovery tolerates per-interface failures too. It +// skips, naming why, only when none could send for lack of a multicast route +// (or there is no eligible interface), and fails when none could send and an +// error was anything else. +func decideMulticast(probes []interfaceProbe) (skip string, failed error, notes []string) { if len(probes) == 0 { - return "no up, multicast-capable IPv4 interface", nil + return "no up, multicast-capable IPv4 interface", nil, nil } + sent := false + var unexpected error var reasons []string for _, p := range probes { if p.err == nil { - return "", nil + sent = true + continue } + reasons = append(reasons, fmt.Sprintf("%s: %v", p.name, p.err)) if !noMulticastRoute(p.err) { - failed = errors.Join(failed, fmt.Errorf("%s: %w", p.name, p.err)) + unexpected = errors.Join(unexpected, fmt.Errorf("%s: %w", p.name, p.err)) + notes = append(notes, fmt.Sprintf("unexpected send error on %s: %v", p.name, p.err)) } - reasons = append(reasons, fmt.Sprintf("%s: %v", p.name, p.err)) } - if failed != nil { - return "", failed + switch { + case sent: + return "", nil, notes + case unexpected != nil: + return "", unexpected, notes + default: + return strings.Join(reasons, "; "), nil, notes } - return strings.Join(reasons, "; "), nil } // noMulticastRoute reports whether a send failed because the host has no @@ -145,20 +163,23 @@ func TestDecideMulticast(t *testing.T) { noRoute := &net.OpError{Op: "write", Net: "udp4", Err: syscall.EHOSTUNREACH} odd := &net.OpError{Op: "write", Net: "udp4", Err: syscall.EPERM} for _, tc := range []struct { - name string - probes []interfaceProbe - wantSkip bool - wantFail bool + name string + probes []interfaceProbe + wantSkip bool + wantFail bool + wantNotes int }{ - {"no eligible interface", nil, true, false}, - {"one interface can send", []interfaceProbe{{"en0", nil}}, false, false}, - {"any interface sending wins", []interfaceProbe{{"en0", noRoute}, {"en1", nil}}, false, false}, - {"no route anywhere", []interfaceProbe{{"en0", noRoute}, {"utun3", noRoute}}, true, false}, - {"an unexpected error fails", []interfaceProbe{{"en0", noRoute}, {"en1", odd}}, false, true}, + {"no eligible interface", nil, true, false, 0}, + {"one interface can send", []interfaceProbe{{"en0", nil}}, false, false, 0}, + {"any interface sending wins", []interfaceProbe{{"en0", noRoute}, {"en1", nil}}, false, false, 0}, + {"no route anywhere", []interfaceProbe{{"en0", noRoute}, {"utun3", noRoute}}, true, false, 0}, + {"an unexpected error with no sender fails", []interfaceProbe{{"en0", noRoute}, {"en1", odd}}, false, true, 1}, + {"success then unexpected error is noted", []interfaceProbe{{"en0", nil}, {"en1", odd}}, false, false, 1}, + {"unexpected error then success is noted", []interfaceProbe{{"en1", odd}, {"en0", nil}}, false, false, 1}, } { - skip, failed := decideMulticast(tc.probes) - if (skip != "") != tc.wantSkip || (failed != nil) != tc.wantFail { - t.Errorf("%s: skip=%q failed=%v", tc.name, skip, failed) + skip, failed, notes := decideMulticast(tc.probes) + if (skip != "") != tc.wantSkip || (failed != nil) != tc.wantFail || len(notes) != tc.wantNotes { + t.Errorf("%s: skip=%q failed=%v notes=%q", tc.name, skip, failed, notes) } } }