From 304c453e03b2b7e09abb0b2c870fb3089ceebf78 Mon Sep 17 00:00:00 2001 From: Jared Ping <13906915+JaredP94@users.noreply.github.com> Date: Fri, 6 Nov 2026 12:00:16 +0200 Subject: [PATCH 1/7] fix(engine-manager): reject stale oMLX launchers Require the app CLI target as well as a declared launcher. Missing installations keep saved ON intent but are skipped at launch, leaving Install available in desktop and TUI without modifying user files. Bump engine-manager from released 1.4.0 to 1.4.1 for these fixes. Signed-off-by: Jared Ping <13906915+JaredP94@users.noreply.github.com> --- docs/known-issues.mdx | 8 ---- services/nvpair-engine-manager/README.md | 9 +++- .../cli_resolution_test.go | 7 ++++ .../nvpair-engine-manager/desiredstate.go | 4 ++ .../nvpair-engine-manager/manifests/omlx.json | 1 + .../omlx_presence_test.go | 41 +++++++++++++++++++ services/nvpair-engine-manager/registry.go | 13 +++--- services/nvpair-engine-manager/resolve.go | 10 +++++ 8 files changed, 79 insertions(+), 14 deletions(-) create mode 100644 services/nvpair-engine-manager/omlx_presence_test.go diff --git a/docs/known-issues.mdx b/docs/known-issues.mdx index 11faae7b8..834a0b963 100644 --- a/docs/known-issues.mdx +++ b/docs/known-issues.mdx @@ -120,14 +120,6 @@ On the peer that needs to connect, add a reachable IP address or hostname as **PAIR machine**, such as its Tailscale address; a saved address does not replace pairing or verification. -## Leftover oMLX Launchers Count as Installed - -A leftover `~/.omlx` launcher can make PAIR report oMLX as installed even when -`/Applications/oMLX.app` is absent, and PAIR retries starting it on each launch. -This also affects 0.8.0. Restore the oMLX app or use the vendor's cleanup -instructions for a stale installation; PAIR's installed status alone does not -prove the app bundle is present. - ## Ollama Install Download Is Not Checksum-Pinned PAIR installs Ollama from the vendor's floating latest-download URL without a diff --git a/services/nvpair-engine-manager/README.md b/services/nvpair-engine-manager/README.md index 97ff1b3ee..a215e360f 100644 --- a/services/nvpair-engine-manager/README.md +++ b/services/nvpair-engine-manager/README.md @@ -109,8 +109,15 @@ On macOS arm64, the bundled manifest checks these CLI paths in order: 2. `/opt/homebrew/bin/omlx` 3. `/usr/local/bin/omlx` +The manifest also requires `/Applications/oMLX.app/Contents/MacOS/omlx-cli`, +the target of the app's launcher (`detect_requires`). A stale launcher without +that target reports **not installed**; saved ON intent is retained but skipped +at launch without an error. In desktop, use **Install oMLX**; in TUI Engines, +select oMLX and press **i** to reinstall the app. PAIR does not delete or alter +`~/.omlx` to repair detection. + The Homebrew and `/usr/local` candidates are assumed to provide the same `omlx` -CLI and `start`/`stop` semantics as the install.sh installation. This change does +CLI and `start`/`stop` semantics as the app's launcher. This change does not verify vendor CLI behavior. No app-bundle executable is a candidate: `oMLX.app/Contents/MacOS/oMLX` is the GUI executable (also matched by lowercase `omlx` on case-insensitive macOS filesystems), and `omlx-cli` and other bundle diff --git a/services/nvpair-engine-manager/cli_resolution_test.go b/services/nvpair-engine-manager/cli_resolution_test.go index b81237de4..2840a871c 100644 --- a/services/nvpair-engine-manager/cli_resolution_test.go +++ b/services/nvpair-engine-manager/cli_resolution_test.go @@ -31,6 +31,10 @@ func isolatedCommandRegistry(t *testing.T, engine, root string) *Registry { plat.Detect[i] = filepath.Join(root, path) } } + for i, path := range plat.DetectRequires { + plat.DetectRequires[i] = filepath.Join(root, path) + resolutionFile(t, plat.DetectRequires[i]) + } // Presence/HTTP adoption is covered separately. Keep these filesystem tests // independent of any real service on the engine's default port. plat.Runtime.Port, plat.Runtime.Ready, plat.Runtime.Health = 0, nil, nil @@ -146,6 +150,9 @@ func TestCommandCLIOverrideIsAuthoritative(t *testing.T) { // shipped macOS platform on any test host and disable real probes. m, _ := reg.Get("omlx") p, _ := m.PlatformFor("darwin", "arm64") + for i, path := range p.DetectRequires { + p.DetectRequires[i] = filepath.Join(home, path) + } p.Runtime.Port, p.Runtime.Ready, p.Runtime.Health = 0, nil, nil m.Platforms = map[string]Platform{runtime.GOOS + "/" + runtime.GOARCH: *p} want := "" diff --git a/services/nvpair-engine-manager/desiredstate.go b/services/nvpair-engine-manager/desiredstate.go index 8c0ef13b0..944b1e560 100644 --- a/services/nvpair-engine-manager/desiredstate.go +++ b/services/nvpair-engine-manager/desiredstate.go @@ -153,5 +153,9 @@ func (e *Executor) restoreEnabled(ctx context.Context, engine string) error { if err != nil || !known || !enabled { return err } + // Retain ON intent for a future reinstall, but never launch a missing engine. + if installed, err := e.Detect(engine); err != nil || !installed { + return err + } return e.doStart(ctx, st, engine, startOpts{}) } diff --git a/services/nvpair-engine-manager/manifests/omlx.json b/services/nvpair-engine-manager/manifests/omlx.json index 3da126a53..e95e7b3b7 100644 --- a/services/nvpair-engine-manager/manifests/omlx.json +++ b/services/nvpair-engine-manager/manifests/omlx.json @@ -7,6 +7,7 @@ "/opt/homebrew/bin/omlx", "/usr/local/bin/omlx" ], + "detect_requires": ["/Applications/oMLX.app/Contents/MacOS/omlx-cli"], "install": { "fetch_variants": [ { diff --git a/services/nvpair-engine-manager/omlx_presence_test.go b/services/nvpair-engine-manager/omlx_presence_test.go new file mode 100644 index 000000000..fdf7d61ab --- /dev/null +++ b/services/nvpair-engine-manager/omlx_presence_test.go @@ -0,0 +1,41 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "context" + "os" + "path/filepath" + "runtime" + "testing" +) + +func TestOmlxStaleLauncherDoesNotRestore(t *testing.T) { + base := resolutionRoot(t) + home, _ := os.UserHomeDir() + reg := isolatedCommandRegistry(t, "omlx", home) + cli := filepath.Join(home, ".omlx/bin/omlx") + resolutionFile(t, cli) + target := filepath.Join(home, "Applications/oMLX.app/Contents/MacOS/omlx-cli") + // Remove only our fixture, never the real app or the user's launcher. + _ = os.Remove(target) + assertCommandPresence(t, reg, base, "omlx", "", false) + ex := reconciledTestExecutor(NewExecutor(reg, NewReporter(nil), nil, base)) + if err := ex.setDesiredEnabled("omlx", true); err != nil { + t.Fatal(err) + } + if err := ex.RestoreEnabled(context.Background()); err != nil { + t.Fatalf("missing app must not produce a launch error: %v", err) + } + if !fileExists(cli) { + t.Fatal("stale launcher was removed") + } + resolutionFile(t, target) + assertCommandPresence(t, reg, base, "omlx", cli, false) + m, _ := reg.Get("omlx") + p, _ := m.PlatformFor(runtime.GOOS, runtime.GOARCH) + if got := detectedExecutable(p, filepath.Join(base, "omlx"), runtime.GOOS); got != cli { + t.Fatalf("restored app: got %q", got) + } +} diff --git a/services/nvpair-engine-manager/registry.go b/services/nvpair-engine-manager/registry.go index 4a979af40..9bab2ac09 100644 --- a/services/nvpair-engine-manager/registry.go +++ b/services/nvpair-engine-manager/registry.go @@ -61,10 +61,12 @@ type Manifest struct { // first; OS-specific Go code is the fallback only for primitives a // manifest cannot express (process termination, console hiding). type Platform struct { - Detect []string `json:"detect,omitempty"` - Install *Install `json:"install,omitempty"` - Uninstall *Uninstall `json:"uninstall,omitempty"` - Runtime Runtime `json:"runtime"` + Detect []string `json:"detect,omitempty"` + // DetectRequires lists files that must also exist (e.g. a launcher target). + DetectRequires []string `json:"detect_requires,omitempty"` + Install *Install `json:"install,omitempty"` + Uninstall *Uninstall `json:"uninstall,omitempty"` + Runtime Runtime `json:"runtime"` // ModelsDir is the engine's on-disk model store, referenced elsewhere as // {models_dir}. Nothing this service removes may delete it: Uninstall // skips it, and validation rejects a removal at or inside it. It must be a @@ -546,7 +548,7 @@ func applyPlatformDefaults(data []byte) ([]byte, error) { return nil, fmt.Errorf("invalid JSON: %w", err) } base := map[string]any{} - for _, k := range []string{"detect", "install", "uninstall", "runtime", "models_dir"} { + for _, k := range []string{"detect", "detect_requires", "install", "uninstall", "runtime", "models_dir"} { raw, ok := top[k] if !ok { continue @@ -903,6 +905,7 @@ func (m *Manifest) templatedStrings() []string { var out []string for _, p := range m.Platforms { out = append(out, p.Detect...) + out = append(out, p.DetectRequires...) if p.Install != nil { out = append(out, p.Install.Run...) out = append(out, p.Install.Script...) diff --git a/services/nvpair-engine-manager/resolve.go b/services/nvpair-engine-manager/resolve.go index 62ce1a0f2..a6b450b44 100644 --- a/services/nvpair-engine-manager/resolve.go +++ b/services/nvpair-engine-manager/resolve.go @@ -33,6 +33,16 @@ type listedEngine struct { // An explicit runtime.cli is authoritative for command mode; otherwise detect // declares the ordered CLI candidates. No PATH lookup or undeclared locations. func detectedExecutable(plat *Platform, installDir, goos string) string { + for _, path := range plat.DetectRequires { + resolved, err := resolvePlaceholders(path, map[string]string{"install_dir": installDir}) + if err != nil { + return "" + } + info, err := os.Stat(expandPathForOS(resolved, goos)) + if err != nil || info.IsDir() { + return "" + } + } paths := plat.Detect if plat.Runtime.modeOrDefault() == "command" && plat.Runtime.CLI != "" { paths = []string{plat.Runtime.CLI} From 66787bb819bacdfcdb81828f9c4bf259269b0530 Mon Sep 17 00:00:00 2001 From: Jared Ping <13906915+JaredP94@users.noreply.github.com> Date: Fri, 6 Nov 2026 12:16:21 +0200 Subject: [PATCH 2/7] fix(engine-manager): wait for command engine teardown A control CLI can close its listener before the serving process finishes tearing down. Capture that PID before stop and require its exit as well as consecutive connection refusals within the existing stop grace, without holding the state mutex. Preserve running state on timeout rather than allowing immediate restart to race pending shutdown. Signed-off-by: Jared Ping <13906915+JaredP94@users.noreply.github.com> --- services/nvpair-engine-manager/README.md | 9 +++ services/nvpair-engine-manager/lifecycle.go | 47 ++++++++--- .../shutdown_tail_test.go | 80 +++++++++++++++++++ .../testdata/fakeengine/main.go | 25 ++++++ 4 files changed, 149 insertions(+), 12 deletions(-) create mode 100644 services/nvpair-engine-manager/shutdown_tail_test.go diff --git a/services/nvpair-engine-manager/README.md b/services/nvpair-engine-manager/README.md index a215e360f..45a76f8ce 100644 --- a/services/nvpair-engine-manager/README.md +++ b/services/nvpair-engine-manager/README.md @@ -561,6 +561,15 @@ SIGKILL escalation), and `taskkill /T /F` on Windows — where the windowless engines we spawn can't receive a graceful (non-`/F`) close, so a forced terminate is the only signal that actually stops them. +Command-mode stop (including oMLX) gives the official stop CLI +`max(grace_s, 5s)` to return, then up to `grace_s` (default 5s) for shutdown. +It confirms three consecutive connection refusals **and**, when the host can +resolve it, exit of the process observed on the port before stop. Closing the +port alone does not mean app teardown is complete. The state mutex is never +held across these waits; lifecycle operations remain serialized so an immediate +start cannot overtake stop. A timeout leaves the engine running, not falsely +stopped, and is returned to the caller. + ### Adoption — start may attach to an engine it didn't launch Before spawning, `engine:start` **probes the chosen port's readiness diff --git a/services/nvpair-engine-manager/lifecycle.go b/services/nvpair-engine-manager/lifecycle.go index ea047bdcb..ba7cc8489 100644 --- a/services/nvpair-engine-manager/lifecycle.go +++ b/services/nvpair-engine-manager/lifecycle.go @@ -28,6 +28,8 @@ const ( engineIdentityProbeHeader = "X-NVPAIR-Engine-Identity-Probe" ) +var errCommandStillRunning = errors.New("engine shutdown is not confirmed") + type listenerProbeResult uint8 const ( @@ -326,7 +328,7 @@ func (e *Executor) bringUpCommand(ctx context.Context, st *engineState, engine s if stopErr := e.runCommandStop(st, engine, rt, port); stopErr != nil { // A non-zero stop CLI can still have taken the daemon down. Confirm // that before surfacing a cleanup failure. - if rt.Ready != nil && e.waitUnavailable(rt.Ready, port, time.Second) { + if !errors.Is(stopErr, errCommandStillRunning) && rt.Ready != nil && e.waitUnavailable(rt.Ready, port, time.Second) { return startErr } return errors.Join(startErr, fmt.Errorf("clean up failed %s start: %w", engine, stopErr)) @@ -527,7 +529,7 @@ func (e *Executor) doStop(st *engineState, engine string) error { return e.reconcileFailedCommandStop(st, engine, true, fmt.Errorf("cannot stop engine %q: no stop command is configured", engine)) } if err := e.runCommandStop(st, engine, rt, port); err != nil { - return e.reconcileFailedCommandStop(st, engine, rt.Ready == nil || !e.waitUnavailable(rt.Ready, port, time.Second), err) + return e.reconcileFailedCommandStop(st, engine, errors.Is(err, errCommandStillRunning) || rt.Ready == nil || !e.waitUnavailable(rt.Ready, port, time.Second), err) } } else if proc != nil { proc.stop() @@ -559,21 +561,31 @@ func (e *Executor) runCommandStop(st *engineState, engine string, rt Runtime, po // Bound the stop CLI (e.g. `lms server stop`): the broker no longer // force-kills engine-manager on a timeout, so an unbounded stop command // would wedge StopAll and, in turn, the whole app shutdown. + // Snapshot the serving PID before the CLI can close its listener. The + // daemon can remain alive tearing down state after the port is released. + // No state mutex is held during lookup, command execution, or this wait; + // opMu still serializes lifecycle operations, as it does for readiness. + pid := 0 + if rt.Ready != nil { + pid, _, _ = pidOnPort(port) + // Never wait for this service itself to exit (e.g. a misconfigured + // probe targeting engine-manager). Only an independent daemon counts. + if pid == os.Getpid() { + pid = 0 + } + } stopCtx, cancelStop := context.WithTimeout(context.Background(), commandStopTimeout(rt)) runErr := e.runCommand(stopCtx, argv) cancelStop() - if runErr != nil { - return fmt.Errorf("stop %s: %w", engine, runErr) - } if rt.Ready != nil { - grace := 5 * time.Second - if sp.GraceS > 0 { - grace = time.Duration(sp.GraceS) * time.Second - } - if !e.waitUnavailable(rt.Ready, port, grace) { - return fmt.Errorf("stop %s: engine is still serving on port %d after %s", engine, port, grace) + grace := stopGrace(rt) + if !e.waitCommandUnavailable(rt.Ready, port, pid, grace) { + return fmt.Errorf("stop %s: engine is still serving or shutting down on port %d after %s: %w", engine, port, grace, errCommandStillRunning) } } + if runErr != nil { + return fmt.Errorf("stop %s: %w", engine, runErr) + } return nil } @@ -633,12 +645,23 @@ func (e *Executor) waitUnavailable(p *Probe, port int, timeout time.Duration) bo } func (e *Executor) waitUnavailableContext(parent context.Context, p *Probe, port int, timeout time.Duration) bool { + return e.waitDownContext(parent, p, port, 0, timeout) +} + +// waitCommandUnavailable requires both transport closure and exit of the PID +// observed before stop. If the host cannot resolve a PID, transport remains +// the best available evidence; the deadline is never extended by polling. +func (e *Executor) waitCommandUnavailable(p *Probe, port, pid int, timeout time.Duration) bool { + return e.waitDownContext(context.Background(), p, port, pid, timeout) +} + +func (e *Executor) waitDownContext(parent context.Context, p *Probe, port, pid int, timeout time.Duration) bool { ctx, cancel := context.WithTimeout(parent, timeout) defer cancel() misses := 0 for { result := probeListener(ctx, p, port) - if result == listenerProbeRefused { + if result == listenerProbeRefused && !pidAlive(pid) { misses++ if misses == unavailableConfirmations { return true diff --git a/services/nvpair-engine-manager/shutdown_tail_test.go b/services/nvpair-engine-manager/shutdown_tail_test.go new file mode 100644 index 000000000..620b05691 --- /dev/null +++ b/services/nvpair-engine-manager/shutdown_tail_test.go @@ -0,0 +1,80 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "errors" + "os/exec" + "path/filepath" + "strconv" + "testing" + "time" +) + +func TestCommandStopWaitsForProcessExitAfterPortCloses(t *testing.T) { + testCommandStopTail(t, "900ms", 2, false) +} + +func TestCommandStopRetainsRunningOnTeardownTimeout(t *testing.T) { + testCommandStopTail(t, "3s", 1, true) +} + +func testCommandStopTail(t *testing.T, delay string, grace int, wantTimeout bool) { + t.Helper() + marker := filepath.Join(t.TempDir(), "stop") + port, err := freePort() + if err != nil { + t.Fatal(err) + } + cmd := exec.Command(fakeEngineBin, "shutdown-tail", marker, "127.0.0.1:"+strconv.Itoa(port), delay) + configureSysProcAttr(cmd) + if err := cmd.Start(); err != nil { + t.Fatal(err) + } + done := make(chan struct{}) + go func() { _ = cmd.Wait(); close(done) }() + t.Cleanup(func() { _ = signalPID(cmd.Process.Pid, true); <-done }) + waitPortServing(t, port) + if pid, _, ok := pidOnPort(port); !ok || pid != cmd.Process.Pid { + t.Skip("host cannot resolve listener PID") + } + m := testEngineManifest(fakeEngineBin) + p := m.Platforms[hostKey()] + p.Runtime.Mode = "command" + p.Runtime.Port = port + p.Runtime.Stop = &StopSpec{Cmd: []string{fakeEngineBin, "touch", marker}, GraceS: grace} + p.Runtime.Health = nil + m.Platforms[hostKey()] = p + ex := newTestExecutor(t, m) + st, _ := ex.state("fake") + st.running, st.port = true, port + started := time.Now() + err = ex.Stop("fake") + if wantTimeout { + if !errors.Is(err, errCommandStillRunning) { + t.Fatalf("expected bounded shutdown timeout, got %v", err) + } + st.mu.Lock() + running := st.running + st.mu.Unlock() + if !running { + t.Fatal("timeout must not report stopped") + } + return + } + if err != nil { + t.Fatal(err) + } + if elapsed := time.Since(started); elapsed > 5*time.Second { + t.Fatalf("stop exceeded lifecycle bound: %s", elapsed) + } + select { + case <-done: + default: + t.Fatal("Stop reported stopped with the port closed but the engine process still shutting down; immediate restart would race") + } + if portServing(port) { + t.Fatal("stop left the port occupied") + } +} diff --git a/services/nvpair-engine-manager/testdata/fakeengine/main.go b/services/nvpair-engine-manager/testdata/fakeengine/main.go index 499c5c4c1..dd241f3e3 100644 --- a/services/nvpair-engine-manager/testdata/fakeengine/main.go +++ b/services/nvpair-engine-manager/testdata/fakeengine/main.go @@ -180,6 +180,31 @@ func main() { os.Exit(1) } return + case "shutdown-tail": + // Close the listener before asynchronous process teardown completes. + ln, err := net.Listen("tcp", os.Args[3]) + if err != nil { + os.Exit(2) + } + go func() { + for { + conn, err := ln.Accept() + if err != nil { + return + } + _ = conn.Close() + } + }() + for { + if _, err := os.Stat(os.Args[2]); err == nil { + break + } + time.Sleep(10 * time.Millisecond) + } + _ = ln.Close() + delay, _ := time.ParseDuration(os.Args[4]) + time.Sleep(delay) + return case "touch": // write a marker file so a test can assert the command ran if len(os.Args) > 2 { _ = os.WriteFile(os.Args[2], []byte("ok"), 0o644) From c7e65dcb32a35f35d657e2057754d80362d18a46 Mon Sep 17 00:00:00 2001 From: Jared Ping <13906915+JaredP94@users.noreply.github.com> Date: Fri, 6 Nov 2026 12:16:59 +0200 Subject: [PATCH 3/7] fix(engine-manager): pin Ollama release downloads Use Ollama v0.40.15, the newest published release, for every bundled platform. The five artifact hashes come from its GitHub release sha256sum.txt and match fresh downloads independently hashed without execution. Initial installation is pinned; vendor self-updates are unchanged. Signed-off-by: Jared Ping <13906915+JaredP94@users.noreply.github.com> --- docs/known-issues.mdx | 8 ---- services/nvpair-engine-manager/README.md | 14 +++++++ .../manifests/ollama.json | 12 +++--- .../ollama_install_test.go | 40 +++++++++++++++++++ 4 files changed, 60 insertions(+), 14 deletions(-) create mode 100644 services/nvpair-engine-manager/ollama_install_test.go diff --git a/docs/known-issues.mdx b/docs/known-issues.mdx index 834a0b963..ed9a00caf 100644 --- a/docs/known-issues.mdx +++ b/docs/known-issues.mdx @@ -120,14 +120,6 @@ On the peer that needs to connect, add a reachable IP address or hostname as **PAIR machine**, such as its Tailscale address; a saved address does not replace pairing or verification. -## Ollama Install Download Is Not Checksum-Pinned - -PAIR installs Ollama from the vendor's floating latest-download URL without a -pinned checksum and logs `UNPINNED download`. This is unchanged since 0.8.0; -download progress is not an integrity check. If a pinned artifact is required, -install an independently verified Ollama release yourself instead of using -PAIR's install action. - ## The Terminal Interface Cannot Do Everything the Desktop App Can **Implemented in source, pending release testing.** `nvpair-tui` now shows diff --git a/services/nvpair-engine-manager/README.md b/services/nvpair-engine-manager/README.md index 45a76f8ce..4a10bdd91 100644 --- a/services/nvpair-engine-manager/README.md +++ b/services/nvpair-engine-manager/README.md @@ -144,6 +144,20 @@ The bundled uninstalls remove only what an install put down: Ollama's app or install directory, and LM Studio's `~/.lmstudio/bin` and `llmster` runtime. LM Studio's conversations, credentials and settings are left where they are. +### Ollama: pinned initial download + +Fresh installs use Ollama **v0.40.15** from +[its GitHub release](https://github.com/ollama/ollama/releases/tag/v0.40.15), +not a floating latest URL. Each SHA-256 comes from that release's +[`sha256sum.txt`](https://github.com/ollama/ollama/releases/download/v0.40.15/sha256sum.txt): +`Ollama-darwin.zip` (both Mac architectures), +`ollama-windows-amd64.zip`, `ollama-windows-arm64.zip`, +`ollama-linux-amd64.tar.zst`, and `ollama-linux-arm64.tar.zst`. +All five artifacts were independently downloaded into a fresh temporary directory +and hashed without executing or installing them. v0.40.15 exceeds the 0.14.0 +minimum needed for `/v1/messages`. Ollama's own updates continue as before; +the pin only selects the initial install, not a permanent runtime version. + ### oMLX: the release disk image oMLX installs the way oMLX recommends: the `.dmg` from its GitHub releases, diff --git a/services/nvpair-engine-manager/manifests/ollama.json b/services/nvpair-engine-manager/manifests/ollama.json index 76a69c4aa..5acb620c5 100644 --- a/services/nvpair-engine-manager/manifests/ollama.json +++ b/services/nvpair-engine-manager/manifests/ollama.json @@ -24,7 +24,7 @@ "windows/amd64": { "detect": ["{install_dir}\\ollama.exe", "%LOCALAPPDATA%\\Programs\\Ollama\\ollama.exe"], "install": { - "fetch": { "url": "https://ollama.com/download/ollama-windows-amd64.zip" }, + "fetch": { "url": "https://github.com/ollama/ollama/releases/download/v0.40.15/ollama-windows-amd64.zip", "sha256": "9ca74a7ab66ec86870b1eecac7df160df08dc4e3a291ab5d2d59bcf9310221c6" }, "run": ["tar", "-xf", "{download}", "-C", "{install_dir}"] }, "uninstall": { "remove": ["{install_dir}"] }, @@ -33,7 +33,7 @@ "windows/arm64": { "detect": ["{install_dir}\\ollama.exe", "%LOCALAPPDATA%\\Programs\\Ollama\\ollama.exe"], "install": { - "fetch": { "url": "https://ollama.com/download/ollama-windows-arm64.zip" }, + "fetch": { "url": "https://github.com/ollama/ollama/releases/download/v0.40.15/ollama-windows-arm64.zip", "sha256": "35ce5195b57496dbc9ecbaa6593807e6ac26cbb53cf1f8bb57ac58daafa58574" }, "run": ["tar", "-xf", "{download}", "-C", "{install_dir}"] }, "uninstall": { "remove": ["{install_dir}"] }, @@ -46,7 +46,7 @@ "{install_dir}/Ollama.app/Contents/Resources/ollama" ], "install": { - "fetch": { "url": "https://ollama.com/download/Ollama-darwin.zip" }, + "fetch": { "url": "https://github.com/ollama/ollama/releases/download/v0.40.15/Ollama-darwin.zip", "sha256": "de3298b9897828480c361c101e3efd2d3c499d08882c8ee972df524064abc3db" }, "run": ["unzip", "-o", "{download}", "-d", "{install_dir}"] }, "uninstall": { "remove": ["{install_dir}/Ollama.app"] }, @@ -59,7 +59,7 @@ "{install_dir}/Ollama.app/Contents/Resources/ollama" ], "install": { - "fetch": { "url": "https://ollama.com/download/Ollama-darwin.zip" }, + "fetch": { "url": "https://github.com/ollama/ollama/releases/download/v0.40.15/Ollama-darwin.zip", "sha256": "de3298b9897828480c361c101e3efd2d3c499d08882c8ee972df524064abc3db" }, "run": ["unzip", "-o", "{download}", "-d", "{install_dir}"] }, "uninstall": { "remove": ["{install_dir}/Ollama.app"] }, @@ -68,7 +68,7 @@ "linux/amd64": { "detect": ["{install_dir}/bin/ollama"], "install": { - "fetch": { "url": "https://ollama.com/download/ollama-linux-amd64.tar.zst" }, + "fetch": { "url": "https://github.com/ollama/ollama/releases/download/v0.40.15/ollama-linux-amd64.tar.zst", "sha256": "628c018404a4206493efd38e46251c154ede5141b8b8dc85c659b3b027c47379" }, "run": ["tar", "--zstd", "-xf", "{download}", "-C", "{install_dir}"] }, "uninstall": { "remove": ["{install_dir}"] }, @@ -80,7 +80,7 @@ "linux/arm64": { "detect": ["{install_dir}/bin/ollama"], "install": { - "fetch": { "url": "https://ollama.com/download/ollama-linux-arm64.tar.zst" }, + "fetch": { "url": "https://github.com/ollama/ollama/releases/download/v0.40.15/ollama-linux-arm64.tar.zst", "sha256": "7f699ead5cdb4ae1ed89b888b148a238af188708800871e1889ab9dcee95f247" }, "run": ["tar", "--zstd", "-xf", "{download}", "-C", "{install_dir}"] }, "uninstall": { "remove": ["{install_dir}"] }, diff --git a/services/nvpair-engine-manager/ollama_install_test.go b/services/nvpair-engine-manager/ollama_install_test.go new file mode 100644 index 000000000..4f226311a --- /dev/null +++ b/services/nvpair-engine-manager/ollama_install_test.go @@ -0,0 +1,40 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "encoding/hex" + "strings" + "testing" +) + +func TestBundledOllamaDownloadsAreReleasePinned(t *testing.T) { + m := bundledManifestSet(t)["ollama"] + // Published v0.40.15/sha256sum.txt; independently downloaded and hashed. + checksums := map[string]string{ + "Ollama-darwin.zip": "de3298b9897828480c361c101e3efd2d3c499d08882c8ee972df524064abc3db", + "ollama-linux-amd64.tar.zst": "628c018404a4206493efd38e46251c154ede5141b8b8dc85c659b3b027c47379", + "ollama-linux-arm64.tar.zst": "7f699ead5cdb4ae1ed89b888b148a238af188708800871e1889ab9dcee95f247", + "ollama-windows-amd64.zip": "9ca74a7ab66ec86870b1eecac7df160df08dc4e3a291ab5d2d59bcf9310221c6", + "ollama-windows-arm64.zip": "35ce5195b57496dbc9ecbaa6593807e6ac26cbb53cf1f8bb57ac58daafa58574", + } + for platform, p := range m.Platforms { + t.Run(platform, func(t *testing.T) { + f := p.Install.Fetch + const prefix = "https://github.com/ollama/ollama/releases/download/v0.40.15/" + if f == nil || !strings.HasPrefix(f.URL, prefix) { + t.Fatalf("download is not release-pinned: %+v", f) + } + artifact := strings.TrimPrefix(f.URL, prefix) + expected, ok := checksums[artifact] + if !ok || f.SHA256 != expected { + t.Fatalf("checksum for %s = %q; want published %q", artifact, f.SHA256, expected) + } + sum, err := hex.DecodeString(f.SHA256) + if err != nil || len(sum) != 32 { + t.Fatalf("invalid SHA-256: %q", f.SHA256) + } + }) + } +} From f853c3f33abc73d6b94f00b10f6e9cebe3ae2652 Mon Sep 17 00:00:00 2001 From: Jared Ping <13906915+JaredP94@users.noreply.github.com> Date: Fri, 6 Nov 2026 12:36:49 +0200 Subject: [PATCH 4/7] test(engine-manager): sweep owned fake engine processes Give every fake invocation and copied executable a per-test owner, with early PID registration inherited by detached children. Test cleanup kills owned process groups and checks for survivors, and TestMain fails and sweeps any missed registered children or launchers under this run directory. Identity checks keep sweeps scoped to this run. Exercise both fatal-test cleanup and the final missed-cleanup safety net. Signed-off-by: Jared Ping <13906915+JaredP94@users.noreply.github.com> --- services/nvpair-engine-manager/README.md | 12 ++ .../capabilities_test.go | 6 +- .../cli_resolution_test.go | 4 +- .../desiredstate_test.go | 20 +-- .../download_completion_unix_test.go | 2 +- .../download_shutdown_test.go | 2 +- services/nvpair-engine-manager/e2e_test.go | 4 +- .../enginenative_test.go | 8 +- .../nvpair-engine-manager/executor_test.go | 56 ++++---- services/nvpair-engine-manager/launch_test.go | 8 +- .../lifecycle_stop_test.go | 32 ++--- services/nvpair-engine-manager/lmsget_test.go | 16 +-- .../nvpair-engine-manager/lmspull_test.go | 2 +- .../loadprofiles_actions_test.go | 4 +- services/nvpair-engine-manager/main_test.go | 26 +++- .../nvpair-engine-manager/manager_test.go | 4 +- .../model_delete_test.go | 15 ++- services/nvpair-engine-manager/model_test.go | 2 +- services/nvpair-engine-manager/models_test.go | 16 +-- .../process_cleanup_test.go | 94 ++++++++++++++ .../nvpair-engine-manager/progress_test.go | 4 +- .../pull_failure_test.go | 2 +- services/nvpair-engine-manager/pull_test.go | 8 +- .../nvpair-engine-manager/remediation_test.go | 44 +++---- .../reservedport_test.go | 2 +- .../nvpair-engine-manager/setport_test.go | 10 +- .../settings_security_test.go | 4 +- .../nvpair-engine-manager/settings_test.go | 22 ++-- .../shutdown_once_test.go | 10 +- .../shutdown_tail_test.go | 6 +- .../nvpair-engine-manager/shutdown_test.go | 16 +-- services/nvpair-engine-manager/status_test.go | 4 +- .../testdata/fakeengine/main.go | 26 ++++ .../nvpair-engine-manager/testprocess_test.go | 122 ++++++++++++++++++ .../testprocess_unix_test.go | 53 ++++++++ .../testprocess_windows_test.go | 33 +++++ .../uninstall_coverage_test.go | 2 +- .../worker_health_test.go | 2 +- 38 files changed, 535 insertions(+), 168 deletions(-) create mode 100644 services/nvpair-engine-manager/process_cleanup_test.go create mode 100644 services/nvpair-engine-manager/testprocess_test.go create mode 100644 services/nvpair-engine-manager/testprocess_unix_test.go create mode 100644 services/nvpair-engine-manager/testprocess_windows_test.go diff --git a/services/nvpair-engine-manager/README.md b/services/nvpair-engine-manager/README.md index 4a10bdd91..ad867e64b 100644 --- a/services/nvpair-engine-manager/README.md +++ b/services/nvpair-engine-manager/README.md @@ -144,6 +144,18 @@ The bundled uninstalls remove only what an install put down: Ollama's app or install directory, and LM Studio's `~/.lmstudio/bin` and `llmster` runtime. LM Studio's conversations, credentials and settings are left where they are. +### Fake-engine test cleanup + +Each fake-engine executable (including test copies) has a per-test owner under +the run's `nvpair-em-test-*` directory. Before doing any work, the fake records +its PID and executable identity there; detached children inherit registration. +A `t.Cleanup` sweep force-kills owned process groups on Unix (process trees on +Windows) and fails the test if any remain. `TestMain` also checks registered +children and processes whose executables are under this run's directory, sweeps +survivors, and fails the run if a per-test cleanup was missed. Sweeps verify +executable identity before signaling PIDs and never target other test runs. +`TestFakeEngineCleanupOnFailedTest` exercises cleanup after a fatal assertion. + ### Ollama: pinned initial download Fresh installs use Ollama **v0.40.15** from diff --git a/services/nvpair-engine-manager/capabilities_test.go b/services/nvpair-engine-manager/capabilities_test.go index 015e1fcfa..45f4228f4 100644 --- a/services/nvpair-engine-manager/capabilities_test.go +++ b/services/nvpair-engine-manager/capabilities_test.go @@ -134,7 +134,7 @@ func TestSetPortCapabilityMatchesEngineSetPortRules(t *testing.T) { {"external command with an official stop command", Runtime{Mode: "command", Start: [][]string{{"start", "{port}"}}, Stop: &StopSpec{Cmd: []string{"stop"}}}, true, true}, } { t.Run(tc.name, func(t *testing.T) { - m := testEngineManifest(fakeEngineBin) + m := testEngineManifest(fakeEngineForTest(t)) m.Actions = nil // isolate set_port from the action flags for key, p := range m.Platforms { p.Runtime = tc.rt @@ -174,7 +174,7 @@ func TestCapabilitiesUseEffectiveManifest(t *testing.T) { // EngineStatus carries the capabilities: engine:status, engine:get-installed, // and the engine:state-changed notification. func TestCapabilitiesOnEveryStatusPath(t *testing.T) { - m := testEngineManifest(fakeEngineBin) + m := testEngineManifest(fakeEngineForTest(t)) for key, p := range m.Platforms { p.Detect = nil // nothing installed p.Runtime.Ready = nil // no probe, so no listener adoption can occur @@ -229,7 +229,7 @@ func TestCapabilitiesOnSetPortResult(t *testing.T) { // registry with no block for this host: get-installed and status still list it, // and report every flag false rather than a guess from another platform. func TestUnsupportedPlatformReportsNoCapabilities(t *testing.T) { - m := testEngineManifest(fakeEngineBin) + m := testEngineManifest(fakeEngineForTest(t)) m.Platforms = map[string]Platform{ "unsupported/platform": m.Platforms[runtime.GOOS+"/"+runtime.GOARCH], } diff --git a/services/nvpair-engine-manager/cli_resolution_test.go b/services/nvpair-engine-manager/cli_resolution_test.go index 2840a871c..f2ff751e5 100644 --- a/services/nvpair-engine-manager/cli_resolution_test.go +++ b/services/nvpair-engine-manager/cli_resolution_test.go @@ -204,7 +204,7 @@ func TestCommandLifecycleAndActionsExecuteResolvedCLI(t *testing.T) { t.Run(map[bool]string{false: "detected candidate", true: "explicit override"}[override], func(t *testing.T) { root := t.TempDir() cli := filepath.Join(root, "control-cli"+exeExt()) - copyFile(t, fakeEngineBin, cli) + copyFile(t, fakeEngineForTest(t), cli) startMarker, stopMarker := filepath.Join(root, "started"), filepath.Join(root, "stopped") p := Platform{Detect: []string{filepath.Join(root, "missing"), cli}, Runtime: Runtime{ Mode: "command", @@ -228,7 +228,7 @@ func TestCommandLifecycleAndActionsExecuteResolvedCLI(t *testing.T) { if err := os.MkdirAll(filepath.Join(root, "fake"), 0o700); err != nil { t.Fatal(err) } - copyFile(t, fakeEngineBin, filepath.Join(root, "fake", "control-cli"+exeExt())) + copyFile(t, fakeEngineForTest(t), filepath.Join(root, "fake", "control-cli"+exeExt())) } t.Cleanup(ex.StopAll) if err := ex.Start(context.Background(), "fake"); err != nil { diff --git a/services/nvpair-engine-manager/desiredstate_test.go b/services/nvpair-engine-manager/desiredstate_test.go index a1c1a847d..7948beeae 100644 --- a/services/nvpair-engine-manager/desiredstate_test.go +++ b/services/nvpair-engine-manager/desiredstate_test.go @@ -46,13 +46,13 @@ func TestDesiredOnSurvivesShutdownAndExplicitOffDoesNot(t *testing.T) { baseDir := t.TempDir() ctx := context.Background() - first := desiredTestExecutor(t, testEngineManifest(fakeEngineBin), baseDir) + first := desiredTestExecutor(t, testEngineManifest(fakeEngineForTest(t)), baseDir) if err := first.Start(ctx, "fake"); err != nil { t.Fatalf("start: %v", err) } first.StopAll() - second := desiredTestExecutor(t, testEngineManifest(fakeEngineBin), baseDir) + second := desiredTestExecutor(t, testEngineManifest(fakeEngineForTest(t)), baseDir) t.Cleanup(second.StopAll) if err := second.RestoreEnabled(ctx); err != nil { t.Fatalf("restore ON: %v", err) @@ -64,7 +64,7 @@ func TestDesiredOnSurvivesShutdownAndExplicitOffDoesNot(t *testing.T) { t.Fatalf("explicit stop: %v", err) } - third := desiredTestExecutor(t, testEngineManifest(fakeEngineBin), baseDir) + third := desiredTestExecutor(t, testEngineManifest(fakeEngineForTest(t)), baseDir) if err := third.RestoreEnabled(ctx); err != nil { t.Fatalf("restore after OFF: %v", err) } @@ -75,7 +75,7 @@ func TestDesiredOnSurvivesShutdownAndExplicitOffDoesNot(t *testing.T) { func TestRestoreEnabledExcludesUnreconciledEngineWithoutChangingIntent(t *testing.T) { baseDir := t.TempDir() - exec := desiredTestExecutor(t, testEngineManifest(fakeEngineBin), baseDir) + exec := desiredTestExecutor(t, testEngineManifest(fakeEngineForTest(t)), baseDir) t.Cleanup(exec.StopAll) if err := exec.desired.set("fake", true); err != nil { t.Fatal(err) @@ -100,13 +100,13 @@ func TestRestoreEnabledExcludesUnreconciledEngineWithoutChangingIntent(t *testin func TestFailedRestoreRetainsDesiredOnForRetry(t *testing.T) { baseDir := t.TempDir() - seed := desiredTestExecutor(t, testEngineManifest(fakeEngineBin), baseDir) + seed := desiredTestExecutor(t, testEngineManifest(fakeEngineForTest(t)), baseDir) if err := seed.Start(context.Background(), "fake"); err != nil { t.Fatalf("seed start: %v", err) } seed.StopAll() - failing := testEngineManifest(fakeEngineBin) + failing := testEngineManifest(fakeEngineForTest(t)) key := runtime.GOOS + "/" + runtime.GOARCH platform := failing.Platforms[key] platform.Runtime.Args = []string{"noserve"} @@ -116,7 +116,7 @@ func TestFailedRestoreRetainsDesiredOnForRetry(t *testing.T) { t.Fatal("expected restore failure") } - retry := desiredTestExecutor(t, testEngineManifest(fakeEngineBin), baseDir) + retry := desiredTestExecutor(t, testEngineManifest(fakeEngineForTest(t)), baseDir) t.Cleanup(retry.StopAll) if err := retry.RestoreEnabled(context.Background()); err != nil { t.Fatalf("retry restore: %v", err) @@ -147,7 +147,7 @@ func TestDeclinedAdoptedStopStillPersistsOffIntent(t *testing.T) { // The httptest listener runs in this test process, whose image is not the // fake engine binary, so the stop is declined (never terminated). - m := testEngineManifest(fakeEngineBin) + m := testEngineManifest(fakeEngineForTest(t)) key := runtime.GOOS + "/" + runtime.GOARCH p := m.Platforms[key] p.Runtime.Port = port @@ -192,14 +192,14 @@ func TestConfirmedStopPersistsOffDespiteCommandExitError(t *testing.T) { t.Fatal(err) } - manifest := testEngineManifest(fakeEngineBin) + manifest := testEngineManifest(fakeEngineForTest(t)) key := runtime.GOOS + "/" + runtime.GOARCH platform := manifest.Platforms[key] platform.Runtime = Runtime{ Mode: "command", Port: port, Ready: &Probe{HTTP: srv.URL, Status: http.StatusOK}, - Stop: &StopSpec{Cmd: []string{fakeEngineBin, "failmark", marker}, GraceS: 2}, + Stop: &StopSpec{Cmd: []string{fakeEngineForTest(t), "failmark", marker}, GraceS: 2}, } manifest.Platforms[key] = platform diff --git a/services/nvpair-engine-manager/download_completion_unix_test.go b/services/nvpair-engine-manager/download_completion_unix_test.go index 6ebe1d817..ad8efc1b9 100644 --- a/services/nvpair-engine-manager/download_completion_unix_test.go +++ b/services/nvpair-engine-manager/download_completion_unix_test.go @@ -29,7 +29,7 @@ func TestLMSCompletionStopsDownloaderChildAfterLauncherExit(t *testing.T) { } }} go func() { - _, err := runLMSDownloadCommand(ctx, []string{fakeEngineBin, "shutdown-download-fixture", marker, "exit-parent", childMarker}, output) + _, err := runLMSDownloadCommand(ctx, []string{fakeEngineForTest(t), "shutdown-download-fixture", marker, "exit-parent", childMarker}, output) done <- err }() select { diff --git a/services/nvpair-engine-manager/download_shutdown_test.go b/services/nvpair-engine-manager/download_shutdown_test.go index 65dd31795..0e99e84a5 100644 --- a/services/nvpair-engine-manager/download_shutdown_test.go +++ b/services/nvpair-engine-manager/download_shutdown_test.go @@ -77,7 +77,7 @@ func testRunStopsDownloader(t *testing.T, ignore, midCancel, tree, exitEarly, us go func() { defer close(pullDone) _, _ = ex.trackedPull(ctx, "lmstudio", "fixture", "shutdown-pull", true, func(ctx context.Context) (json.RawMessage, error) { - argv := []string{fakeEngineBin, "shutdown-download-fixture", marker, mode} + argv := []string{fakeEngineForTest(t), "shutdown-download-fixture", marker, mode} if tree { argv = append(argv, childMarker) } diff --git a/services/nvpair-engine-manager/e2e_test.go b/services/nvpair-engine-manager/e2e_test.go index 38307e2f3..533ae801c 100644 --- a/services/nvpair-engine-manager/e2e_test.go +++ b/services/nvpair-engine-manager/e2e_test.go @@ -133,7 +133,7 @@ func TestE2EPrepareShutdownCancelsStartingEngine(t *testing.T) { cfg := t.TempDir() home := t.TempDir() pidFile := filepath.Join(t.TempDir(), "fake.pid") - manifest := testEngineManifest(fakeEngineBin) + manifest := testEngineManifest(fakeEngineForTest(t)) platform := manifest.Platforms[hostKey()] platform.Runtime.Env["FAKE_PID_FILE"] = pidFile platform.Runtime.Env["FAKE_START_DELAY"] = "1m" @@ -256,7 +256,7 @@ func (m *e2eManager) stop(t *testing.T) { } func writeFakeManifest(t *testing.T, dir string) { - writeE2EManifest(t, dir, testEngineManifest(fakeEngineBin)) + writeE2EManifest(t, dir, testEngineManifest(fakeEngineForTest(t))) } func writeE2EManifest(t *testing.T, dir string, manifest *Manifest) { diff --git a/services/nvpair-engine-manager/enginenative_test.go b/services/nvpair-engine-manager/enginenative_test.go index f1ba94f50..fac6d5fea 100644 --- a/services/nvpair-engine-manager/enginenative_test.go +++ b/services/nvpair-engine-manager/enginenative_test.go @@ -136,7 +136,7 @@ func TestEngineNativeStatsReportsTheEngineView(t *testing.T) { // with no omlx engine has no admin API to ask. That is a normal state, not a // failure, so the answer is "unsupported" with no error. func TestEngineNativeStatsReportsUnsupportedWhenTheEngineHasNoAdminAPI(t *testing.T) { - ex := newTestExecutor(t, testEngineManifest(fakeEngineBin)) + ex := newTestExecutor(t, testEngineManifest(fakeEngineForTest(t))) got := ex.EngineNativeStats(context.Background()) if got.Supported { t.Fatalf("supported = true, want false (%+v)", got) @@ -330,7 +330,7 @@ func TestEngineNativeActivityReportsTheEngineRows(t *testing.T) { // activity view follows the stats view — no admin API means "unsupported", // with no error. func TestEngineNativeActivityReportsUnsupportedWhenTheEngineHasNoAdminAPI(t *testing.T) { - ex := newTestExecutor(t, testEngineManifest(fakeEngineBin)) + ex := newTestExecutor(t, testEngineManifest(fakeEngineForTest(t))) got := ex.EngineNativeActivity(context.Background()) if got.Supported { t.Fatalf("supported = true, want false (%+v)", got) @@ -347,7 +347,7 @@ func TestEngineNativeActivityReportsUnsupportedWhenTheEngineHasNoAdminAPI(t *tes // ride the em surface, so they are served on the same listener behind the // same two gates — plaintext for loopback only, mTLS for pinned peers only. func TestEngineNativeSurfaceIsGatedLikeTheModelList(t *testing.T) { - ex := newTestExecutor(t, testEngineManifest(fakeEngineBin)) + ex := newTestExecutor(t, testEngineManifest(fakeEngineForTest(t))) plainMux, tlsMux := emSurface(ex, clustertrust.Open(t.TempDir())) for _, path := range []string{engineStatsPath, engineActivityPath} { @@ -408,7 +408,7 @@ func TestEngineNativeSurfaceIsGatedLikeTheModelList(t *testing.T) { // loopback. The mTLS personality does not serve the path at all, so no peer // can read another node's live engine rows. func TestTheLiveActivityPathIsNotServedToPeers(t *testing.T) { - ex := newTestExecutor(t, testEngineManifest(fakeEngineBin)) + ex := newTestExecutor(t, testEngineManifest(fakeEngineForTest(t))) plainMux, tlsMux := emSurface(ex, clustertrust.Open(t.TempDir())) // The local read still works: this node's own proxy reads it over diff --git a/services/nvpair-engine-manager/executor_test.go b/services/nvpair-engine-manager/executor_test.go index 323192de9..f317d43d1 100644 --- a/services/nvpair-engine-manager/executor_test.go +++ b/services/nvpair-engine-manager/executor_test.go @@ -69,7 +69,7 @@ func responseHeaderTimeout(t *testing.T, client *http.Client) time.Duration { } func TestEngineHTTPClientsBoundResponseHeaders(t *testing.T) { - ex := newTestExecutor(t, testEngineManifest(fakeEngineBin)) + ex := newTestExecutor(t, testEngineManifest(fakeEngineForTest(t))) if got := responseHeaderTimeout(t, ex.client); got != engineResponseHeaderTimeout { t.Fatalf("ordinary response-header timeout = %s, want %s", got, engineResponseHeaderTimeout) } @@ -97,7 +97,7 @@ func TestActionDeclaredTimeoutBoundsResponseHeaders(t *testing.T) { u, _ := url.Parse(srv.URL) port, _ := strconv.Atoi(u.Port()) - m := testEngineManifest(fakeEngineBin) + m := testEngineManifest(fakeEngineForTest(t)) m.Engine = "other" platform := m.Platforms[runtime.GOOS+"/"+runtime.GOARCH] platform.Runtime.Port = port @@ -154,7 +154,7 @@ func TestActionTimeoutValidation(t *testing.T) { } func TestEngineLifecycle(t *testing.T) { - ex := newTestExecutor(t, testEngineManifest(fakeEngineBin)) + ex := newTestExecutor(t, testEngineManifest(fakeEngineForTest(t))) ctx := context.Background() t.Cleanup(func() { _ = ex.Stop("fake") }) @@ -163,7 +163,7 @@ func TestEngineLifecycle(t *testing.T) { t.Fatalf("detect: %v", err) } if !installed { - t.Fatalf("expected fake engine to be detected (bin=%s)", fakeEngineBin) + t.Fatalf("expected fake engine to be detected (bin=%s)", fakeEngineForTest(t)) } if err := ex.Start(ctx, "fake"); err != nil { @@ -199,7 +199,7 @@ func TestCrashThenExternalServiceIsAdoptedWithoutRespawn(t *testing.T) { if err != nil { t.Fatal(err) } - m := testEngineManifest(fakeEngineBin) + m := testEngineManifest(fakeEngineForTest(t)) key := runtime.GOOS + "/" + runtime.GOARCH p := m.Platforms[key] p.Runtime.Port = port @@ -248,21 +248,21 @@ func TestCrashThenExternalServiceIsAdoptedWithoutRespawn(t *testing.T) { } func TestActionRequiresRunning(t *testing.T) { - ex := newTestExecutor(t, testEngineManifest(fakeEngineBin)) + ex := newTestExecutor(t, testEngineManifest(fakeEngineForTest(t))) if _, err := ex.Action(context.Background(), "fake", "list_models", nil); err == nil { t.Fatal("expected error: action on a non-running engine") } } func TestStartUnknownEngine(t *testing.T) { - ex := newTestExecutor(t, testEngineManifest(fakeEngineBin)) + ex := newTestExecutor(t, testEngineManifest(fakeEngineForTest(t))) if err := ex.Start(context.Background(), "nope"); err == nil { t.Fatal("expected error for unknown engine") } } func TestStartPortOverride(t *testing.T) { - ex := newTestExecutor(t, testEngineManifest(fakeEngineBin)) // manifest Port is 0 + ex := newTestExecutor(t, testEngineManifest(fakeEngineForTest(t))) // manifest Port is 0 t.Cleanup(func() { _ = ex.Stop("fake") }) const want = 17777 if err := ex.StartWith(context.Background(), "fake", startOpts{Port: want}); err != nil { @@ -506,7 +506,7 @@ func TestCommandModeMissingCLIInstallsDespiteLiveAPI(t *testing.T) { Engine: "missing-cli", DisplayName: "Missing CLI", ManifestVersion: 1, Platforms: map[string]Platform{key: { Detect: []string{cli}, - Install: &Install{Run: []string{fakeEngineBin, "touch", cli}}, + Install: &Install{Run: []string{fakeEngineForTest(t), "touch", cli}}, Runtime: Runtime{ Mode: "command", CLI: cli, Port: port, Ready: &Probe{HTTP: "http://127.0.0.1:{port}/v1/models", Status: http.StatusOK}, @@ -611,7 +611,7 @@ func TestProxyFacadeCannotIdentifyAsOllama(t *testing.T) { u, _ := url.Parse(srv.URL) port, _ := strconv.Atoi(u.Port()) - m := testEngineManifest(fakeEngineBin) + m := testEngineManifest(fakeEngineForTest(t)) m.Engine = "ollama" key := runtime.GOOS + "/" + runtime.GOARCH p := m.Platforms[key] @@ -660,7 +660,7 @@ func TestPreviouslyAdoptedForeignReplacementFailsClosed(t *testing.T) { u, _ := url.Parse(srv.URL) port, _ := strconv.Atoi(u.Port()) - m := testEngineManifest(fakeEngineBin) + m := testEngineManifest(fakeEngineForTest(t)) m.Engine = "ollama" m.DisplayName = "Ollama" key := runtime.GOOS + "/" + runtime.GOARCH @@ -695,7 +695,7 @@ func TestUninstallTerminatesRunningInstance(t *testing.T) { if err := os.MkdirAll(filepath.Dir(bin), 0o755); err != nil { t.Fatal(err) } - copyFile(t, fakeEngineBin, bin) + copyFile(t, fakeEngineForTest(t), bin) port, err := freePort() if err != nil { @@ -707,7 +707,7 @@ func TestUninstallTerminatesRunningInstance(t *testing.T) { if err := cmd.Start(); err != nil { t.Fatal(err) } - t.Cleanup(func() { _ = cmd.Process.Kill() }) + t.Cleanup(func() { killTestProcessGroup(cmd.Process.Pid); _ = cmd.Wait() }) waitPortServing(t, port) if _, image, ok := pidOnPort(port); !ok || image == "" { t.Skip("host cannot resolve the PID/image owning a port; skipping PID-precise stop test") @@ -743,8 +743,8 @@ func TestUninstallDoesNotKillExternalSameNameProcess(t *testing.T) { if err := os.MkdirAll(filepath.Dir(managedBin), 0o755); err != nil { t.Fatal(err) } - copyFile(t, fakeEngineBin, managedBin) - copyFile(t, fakeEngineBin, externalBin) + copyFile(t, fakeEngineForTest(t), managedBin) + copyFile(t, fakeEngineForTest(t), externalBin) port, err := freePort() if err != nil { @@ -756,7 +756,7 @@ func TestUninstallDoesNotKillExternalSameNameProcess(t *testing.T) { if err := cmd.Start(); err != nil { t.Fatal(err) } - t.Cleanup(func() { _ = cmd.Process.Kill() }) + t.Cleanup(func() { killTestProcessGroup(cmd.Process.Pid); _ = cmd.Wait() }) waitPortServing(t, port) m := testEngineManifest(managedBin) @@ -785,7 +785,7 @@ func TestUninstallDoesNotKillExternalSameNameProcess(t *testing.T) { func TestUninstallRefusesExternalProcessEngine(t *testing.T) { externalBin := filepath.Join(t.TempDir(), "external"+exeExt()) - copyFile(t, fakeEngineBin, externalBin) + copyFile(t, fakeEngineForTest(t), externalBin) m := testEngineManifest(externalBin) key := runtime.GOOS + "/" + runtime.GOARCH p := m.Platforms[key] @@ -809,7 +809,7 @@ func TestUninstallRefusesUnidentifiedLiveListener(t *testing.T) { if err := os.MkdirAll(filepath.Dir(managedBin), 0o755); err != nil { t.Fatal(err) } - copyFile(t, fakeEngineBin, managedBin) + copyFile(t, fakeEngineForTest(t), managedBin) srv := httptest.NewServer(http.NotFoundHandler()) defer srv.Close() @@ -868,7 +868,7 @@ func TestDownloadVerify(t *testing.T) { })) defer srv.Close() - ex := newTestExecutor(t, testEngineManifest(fakeEngineBin)) + ex := newTestExecutor(t, testEngineManifest(fakeEngineForTest(t))) ctx := context.Background() p, err := ex.download(ctx, "fake", &Fetch{URL: srv.URL + "/installer.ps1", SHA256: good}) @@ -898,7 +898,7 @@ func TestDownloadVerify(t *testing.T) { } func TestInstallRefusesAdmin(t *testing.T) { - m := testEngineManifest(fakeEngineBin) + m := testEngineManifest(fakeEngineForTest(t)) key := runtime.GOOS + "/" + runtime.GOARCH p := m.Platforms[key] p.Detect = nil // ensure not-detected so install reaches the mode check @@ -920,7 +920,7 @@ func TestEngineUninstall(t *testing.T) { if err := os.MkdirAll(filepath.Dir(binCopy), 0o755); err != nil { t.Fatal(err) } - copyFile(t, fakeEngineBin, binCopy) + copyFile(t, fakeEngineForTest(t), binCopy) m := testEngineManifest(binCopy) key := runtime.GOOS + "/" + runtime.GOARCH @@ -1000,12 +1000,12 @@ func TestCommandModeLifecycle(t *testing.T) { Engine: "daemon", DisplayName: "Daemon Engine", ManifestVersion: 1, Platforms: map[string]Platform{ key: { - Detect: []string{fakeEngineBin}, + Detect: []string{fakeEngineForTest(t)}, Runtime: Runtime{ Mode: "command", Port: port, - Start: [][]string{{fakeEngineBin, "touch", startMarker}}, - Stop: &StopSpec{Cmd: []string{fakeEngineBin, "touch", stopMarker}}, + Start: [][]string{{fakeEngineForTest(t), "touch", startMarker}}, + Stop: &StopSpec{Cmd: []string{fakeEngineForTest(t), "touch", stopMarker}}, Ready: &Probe{HTTP: "http://127.0.0.1:{port}/", Status: 200, TimeoutS: 10}, Health: &Probe{HTTP: "http://127.0.0.1:{port}/", Status: 200, IntervalS: 1}, }, @@ -1040,8 +1040,8 @@ func TestCommandModeLifecycle(t *testing.T) { // TestCmdAction exercises a CLI action with a param placeholder; it must // run without the engine being "running" and return the command stdout. func TestCmdAction(t *testing.T) { - m := testEngineManifest(fakeEngineBin) - m.Actions["echo"] = Action{Cmd: []string{fakeEngineBin, "echo", "{model}"}} + m := testEngineManifest(fakeEngineForTest(t)) + m.Actions["echo"] = Action{Cmd: []string{fakeEngineForTest(t), "echo", "{model}"}} ex := newTestExecutor(t, m) res, err := ex.Action(context.Background(), "fake", "echo", json.RawMessage(`{"model":"llama3"}`)) @@ -1112,6 +1112,10 @@ func copyFile(t *testing.T, src, dst string) { if err := os.WriteFile(dst, data, 0o755); err != nil { t.Fatal(err) } + // Copies must retain test ownership, even outside the run directory. + if isManagedInstallPath(src, testProcessRoot) { + registerFakeExecutable(t, dst, fakeProcessDir(t)) + } } func rmCmd(path string) []string { diff --git a/services/nvpair-engine-manager/launch_test.go b/services/nvpair-engine-manager/launch_test.go index a3ad02355..698c8e3fe 100644 --- a/services/nvpair-engine-manager/launch_test.go +++ b/services/nvpair-engine-manager/launch_test.go @@ -84,7 +84,7 @@ func TestCommandLaunchRejectsEmptyExecutable(t *testing.T) { func TestLaunchTextReachesChildLiterally(t *testing.T) { path := filepath.Join(t.TempDir(), "captured args.json") want := []string{"", "two words", "日本語", `C:\new\tools\`, `\\server\share`, `a"b`, "$HOME", "%USERPROFILE%", "{port}", "$(ignored)", "line\nbreak"} - tokens := append([]string{fakeEngineBin, "captureargs", path}, want...) + tokens := append([]string{fakeEngineForTest(t), "captureargs", path}, want...) text, err := formatLaunchText(tokens) if err != nil { t.Fatal(err) @@ -110,11 +110,11 @@ func TestLifecycleUsesResolvedLaunchBuilder(t *testing.T) { for _, mode := range []string{"process", "command"} { t.Run(mode, func(t *testing.T) { path := filepath.Join(t.TempDir(), "captured args.json") - manifest := testEngineManifest(fakeEngineBin) + manifest := testEngineManifest(fakeEngineForTest(t)) for key, platform := range manifest.Platforms { - platform.Runtime = Runtime{Mode: mode, Bin: fakeEngineBin, Port: 25001, + platform.Runtime = Runtime{Mode: mode, Bin: fakeEngineForTest(t), Port: 25001, Args: []string{"captureargs", path, "{host}", "{port}", "two words", ""}, - Start: [][]string{{fakeEngineBin, "captureargs", path, "{host}", "{port}", "two words", ""}}, + Start: [][]string{{fakeEngineForTest(t), "captureargs", path, "{host}", "{port}", "two words", ""}}, } manifest.Platforms[key] = platform } diff --git a/services/nvpair-engine-manager/lifecycle_stop_test.go b/services/nvpair-engine-manager/lifecycle_stop_test.go index e5e36e8d9..4fa75c0e7 100644 --- a/services/nvpair-engine-manager/lifecycle_stop_test.go +++ b/services/nvpair-engine-manager/lifecycle_stop_test.go @@ -30,7 +30,7 @@ func spawnFakeListener(t *testing.T, binPath string, port int) *exec.Cmd { if err := cmd.Start(); err != nil { t.Fatal(err) } - t.Cleanup(func() { _ = cmd.Process.Kill() }) + t.Cleanup(func() { killTestProcessGroup(cmd.Process.Pid); _ = cmd.Wait() }) waitPortServing(t, port) if _, image, ok := pidOnPort(port); !ok || image == "" { t.Skip("host cannot resolve the PID/image owning a port; skipping PID-precise stop test") @@ -47,7 +47,7 @@ func TestStopReclaimsOurOrphanOnManagedPort(t *testing.T) { if err := os.MkdirAll(filepath.Dir(bin), 0o755); err != nil { t.Fatal(err) } - copyFile(t, fakeEngineBin, bin) + copyFile(t, fakeEngineForTest(t), bin) port, err := freePort() if err != nil { t.Fatal(err) @@ -86,7 +86,7 @@ func TestStopReclaimsOurOrphanOnManagedPort(t *testing.T) { func TestStopDoesNotReclaimSameExternalImage(t *testing.T) { bin := filepath.Join(t.TempDir(), "external-ollama"+strconv.Itoa(os.Getpid())+exeExt()) - copyFile(t, fakeEngineBin, bin) + copyFile(t, fakeEngineForTest(t), bin) port, err := freePort() if err != nil { t.Fatal(err) @@ -118,8 +118,8 @@ func TestStopDeclinesForeignListenerWithActionableError(t *testing.T) { dir := t.TempDir() ourBin := filepath.Join(dir, "ourengine"+exeExt()) foreignBin := filepath.Join(dir, "foreignengine"+exeExt()) - copyFile(t, fakeEngineBin, ourBin) - copyFile(t, fakeEngineBin, foreignBin) + copyFile(t, fakeEngineForTest(t), ourBin) + copyFile(t, fakeEngineForTest(t), foreignBin) port, err := freePort() if err != nil { t.Fatal(err) @@ -163,7 +163,7 @@ func TestStopRejectsAdoptedProcessEngine(t *testing.T) { u, _ := url.Parse(srv.URL) port, _ := strconv.Atoi(u.Port()) - m := testEngineManifest(fakeEngineBin) + m := testEngineManifest(fakeEngineForTest(t)) p := m.Platforms[runtime.GOOS+"/"+runtime.GOARCH] p.Runtime.Port = port p.Runtime.Ready = &Probe{HTTP: srv.URL, Status: http.StatusOK} @@ -202,7 +202,7 @@ func TestStopReconcilesAdoptedProcessAfterExternalStop(t *testing.T) { })) u, _ := url.Parse(srv.URL) port, _ := strconv.Atoi(u.Port()) - m := testEngineManifest(fakeEngineBin) + m := testEngineManifest(fakeEngineForTest(t)) p := m.Platforms[runtime.GOOS+"/"+runtime.GOARCH] p.Runtime.Port = port p.Runtime.Ready = &Probe{HTTP: srv.URL, Status: http.StatusOK} @@ -233,7 +233,7 @@ func TestStopKeepsAdoptedProcessRunningWhileEndpointIsUnhealthy(t *testing.T) { defer srv.Close() u, _ := url.Parse(srv.URL) port, _ := strconv.Atoi(u.Port()) - m := testEngineManifest(fakeEngineBin) + m := testEngineManifest(fakeEngineForTest(t)) p := m.Platforms[runtime.GOOS+"/"+runtime.GOARCH] p.Runtime.Port = port p.Runtime.Ready = &Probe{HTTP: srv.URL, Status: http.StatusOK} @@ -258,7 +258,7 @@ func TestCommandStopFailureKeepsLiveEngineRunning(t *testing.T) { })) defer srv.Close() marker := filepath.Join(t.TempDir(), "stop-attempted") - ex := commandStopTestExecutor(t, srv.URL, []string{fakeEngineBin, "failmark", marker}, 1, false) + ex := commandStopTestExecutor(t, srv.URL, []string{fakeEngineForTest(t), "failmark", marker}, 1, false) err := ex.Stop("command") if err == nil || !strings.Contains(err.Error(), "stop command") { @@ -286,7 +286,7 @@ func TestCommandStopFailureSucceedsWhenEndpointIsConfirmedDown(t *testing.T) { srv.Close() close(closed) }() - ex := commandStopTestExecutor(t, srv.URL, []string{fakeEngineBin, "failmark", marker}, 2, false) + ex := commandStopTestExecutor(t, srv.URL, []string{fakeEngineForTest(t), "failmark", marker}, 2, false) if err := ex.Stop("command"); err != nil { t.Fatalf("nonzero stop command with a closed endpoint must succeed: %v", err) @@ -303,7 +303,7 @@ func TestCommandStopFailureSucceedsWhenEndpointIsConfirmedDown(t *testing.T) { func TestCommandStopWithoutReadinessProbeUsesConfiguredCommand(t *testing.T) { marker := filepath.Join(t.TempDir(), "stop-attempted") - ex := commandStopTestExecutor(t, "http://127.0.0.1:1", []string{fakeEngineBin, "touch", marker}, 1, false) + ex := commandStopTestExecutor(t, "http://127.0.0.1:1", []string{fakeEngineForTest(t), "touch", marker}, 1, false) st, _ := ex.state("command") st.plat.Runtime.Ready = nil @@ -324,7 +324,7 @@ func TestCommandStopRejectsSuccessWhileEndpointIsLive(t *testing.T) { })) defer srv.Close() marker := filepath.Join(t.TempDir(), "stop-attempted") - ex := commandStopTestExecutor(t, srv.URL, []string{fakeEngineBin, "touch", marker}, 1, false) + ex := commandStopTestExecutor(t, srv.URL, []string{fakeEngineForTest(t), "touch", marker}, 1, false) err := ex.Stop("command") if err == nil || !strings.Contains(err.Error(), "still serving") { @@ -341,7 +341,7 @@ func TestCommandStopRejectsSuccessWhileEndpointIsUnhealthy(t *testing.T) { })) defer srv.Close() marker := filepath.Join(t.TempDir(), "stop-attempted") - ex := commandStopTestExecutor(t, srv.URL, []string{fakeEngineBin, "touch", marker}, 1, false) + ex := commandStopTestExecutor(t, srv.URL, []string{fakeEngineForTest(t), "touch", marker}, 1, false) err := ex.Stop("command") if err == nil || !strings.Contains(err.Error(), "still serving") { @@ -357,7 +357,7 @@ func TestCommandStopFailureRestartsHealthMonitoring(t *testing.T) { w.WriteHeader(http.StatusOK) })) marker := filepath.Join(t.TempDir(), "stop-attempted") - ex := commandStopTestExecutor(t, srv.URL, []string{fakeEngineBin, "touch", marker}, 1, false) + ex := commandStopTestExecutor(t, srv.URL, []string{fakeEngineForTest(t), "touch", marker}, 1, false) st, _ := ex.state("command") st.plat.Runtime.Health = &Probe{HTTP: srv.URL, Status: http.StatusOK, IntervalS: 1} t.Cleanup(func() { @@ -397,7 +397,7 @@ func TestCommandStopReportsStoppedOnlyAfterEndpointCloses(t *testing.T) { srv.Close() close(closed) }() - ex := commandStopTestExecutor(t, srv.URL, []string{fakeEngineBin, "touch", marker}, 2, true) + ex := commandStopTestExecutor(t, srv.URL, []string{fakeEngineForTest(t), "touch", marker}, 2, true) if err := ex.Stop("command"); err != nil { t.Fatalf("stop: %v", err) @@ -427,7 +427,7 @@ func commandStopTestExecutor(t *testing.T, readyURL string, stopCmd []string, gr Engine: "command", DisplayName: "Command Engine", ManifestVersion: 1, Platforms: map[string]Platform{ key: { - Detect: []string{fakeEngineBin}, + Detect: []string{fakeEngineForTest(t)}, Runtime: Runtime{ Mode: "command", Port: port, diff --git a/services/nvpair-engine-manager/lmsget_test.go b/services/nvpair-engine-manager/lmsget_test.go index e4b9db6da..22b83ba62 100644 --- a/services/nvpair-engine-manager/lmsget_test.go +++ b/services/nvpair-engine-manager/lmsget_test.go @@ -142,9 +142,9 @@ func TestIsLMSTransientDownloadError(t *testing.T) { // bare Hub id and succeeds for a Hugging Face URL — proving the runner // falls back from Hub to Hugging Face and returns the winning candidate. func TestCmdActionLMSGetFallback(t *testing.T) { - m := testEngineManifest(fakeEngineBin) + m := testEngineManifest(fakeEngineForTest(t)) m.Actions["pull_model"] = Action{ - Cmd: []string{fakeEngineBin, "resolvesim", "{model}"}, + Cmd: []string{fakeEngineForTest(t), "resolvesim", "{model}"}, ModelResolution: modelResolutionLMSGet, } ex := newTestExecutor(t, m) @@ -163,9 +163,9 @@ func TestCmdActionLMSGetFallback(t *testing.T) { // resolve, the action surfaces the (last) resolve error rather than // silently succeeding. func TestCmdActionLMSGetAllFail(t *testing.T) { - m := testEngineManifest(fakeEngineBin) + m := testEngineManifest(fakeEngineForTest(t)) m.Actions["pull_model"] = Action{ - Cmd: []string{fakeEngineBin, "resolvesim", "{model}"}, + Cmd: []string{fakeEngineForTest(t), "resolvesim", "{model}"}, ModelResolution: modelResolutionLMSGet, } ex := newTestExecutor(t, m) @@ -189,9 +189,9 @@ func TestCmdActionLMSGetResumesTransientDownload(t *testing.T) { defer setResumeBudget(t, 3, time.Millisecond)() counter := filepath.Join(t.TempDir(), "n") - m := testEngineManifest(fakeEngineBin) + m := testEngineManifest(fakeEngineForTest(t)) m.Actions["pull_model"] = Action{ - Cmd: []string{fakeEngineBin, "downloadsim", counter, "2", "{model}"}, + Cmd: []string{fakeEngineForTest(t), "downloadsim", counter, "2", "{model}"}, ModelResolution: modelResolutionLMSGet, } ex := newTestExecutor(t, m) @@ -213,9 +213,9 @@ func TestCmdActionLMSGetResumeExhausted(t *testing.T) { defer setResumeBudget(t, 3, time.Millisecond)() counter := filepath.Join(t.TempDir(), "n") - m := testEngineManifest(fakeEngineBin) + m := testEngineManifest(fakeEngineForTest(t)) m.Actions["pull_model"] = Action{ - Cmd: []string{fakeEngineBin, "downloadsim", counter, "99", "{model}"}, + Cmd: []string{fakeEngineForTest(t), "downloadsim", counter, "99", "{model}"}, ModelResolution: modelResolutionLMSGet, } ex := newTestExecutor(t, m) diff --git a/services/nvpair-engine-manager/lmspull_test.go b/services/nvpair-engine-manager/lmspull_test.go index c937fbff1..a06a031c9 100644 --- a/services/nvpair-engine-manager/lmspull_test.go +++ b/services/nvpair-engine-manager/lmspull_test.go @@ -28,7 +28,7 @@ func TestLMSCommandCancellationRequiresConfirmation(t *testing.T) { } }} done := make(chan error, 1) - argv := []string{fakeEngineBin, "download-cancel-fixture"} + argv := []string{fakeEngineForTest(t), "download-cancel-fixture"} if !confirmed { argv = append(argv, "unconfirmed") } diff --git a/services/nvpair-engine-manager/loadprofiles_actions_test.go b/services/nvpair-engine-manager/loadprofiles_actions_test.go index ade09c591..4af49793f 100644 --- a/services/nvpair-engine-manager/loadprofiles_actions_test.go +++ b/services/nvpair-engine-manager/loadprofiles_actions_test.go @@ -25,12 +25,12 @@ func profileActionExecutor(t *testing.T) (*Executor, string, string) { t.Helper() capture := filepath.Join(t.TempDir(), "argv.jsonl") t.Setenv("PAIR_TEST_PROFILE_CAPTURE", capture) - bin := filepath.Join(t.TempDir(), "profile-cli"+filepath.Ext(fakeEngineBin)) + bin := filepath.Join(t.TempDir(), "profile-cli"+filepath.Ext(fakeEngineForTest(t))) if output, err := exec.Command("go", "build", "-o", bin, "./testdata/fakeprofilecli").CombinedOutput(); err != nil { t.Fatalf("build profile CLI fixture: %v %s", err, output) } root := t.TempDir() - m := deleteModelRestartManifest(t, root, fakeEngineBin) + m := deleteModelRestartManifest(t, root, fakeEngineForTest(t)) m.Engine = "lmstudio" m.Actions["load_model"] = Action{Cmd: []string{bin, "load", "{model}"}} m.Actions["unload_model"] = Action{Cmd: []string{bin, "unload", "{model}"}} diff --git a/services/nvpair-engine-manager/main_test.go b/services/nvpair-engine-manager/main_test.go index a451f891d..1c7e30bfa 100644 --- a/services/nvpair-engine-manager/main_test.go +++ b/services/nvpair-engine-manager/main_test.go @@ -4,6 +4,7 @@ package main import ( + "fmt" "os" "os/exec" "path/filepath" @@ -15,8 +16,9 @@ import ( // a real inference engine) and the engine-manager binary itself (for // the cross-process e2e test). var ( - fakeEngineBin string - managerBin string + fakeEngineBin string + managerBin string + testProcessRoot string ) func TestMain(m *testing.M) { @@ -27,6 +29,10 @@ func TestMain(m *testing.M) { if err != nil { panic(err) } + testProcessRoot = tmp + if err := os.Setenv("PAIR_TEST_PROCESS_ROOT", tmp); err != nil { + panic(err) + } suffix := "" if runtime.GOOS == "windows" { suffix = ".exe" @@ -43,6 +49,22 @@ func TestMain(m *testing.M) { } code := m.Run() + // Final safety net for detached children, including a test that missed its + // per-test cleanup. Any survivor here makes the run fail, even if swept. + // Include unregistered launchers that died before the fake could register. + for _, record := range testRunProcesses(tmp) { + fmt.Fprintf(os.Stderr, "test process survived: %+v\n", record) + killTestProcessGroup(record.PID) + code = 1 + } + if alive := trackedFakeProcesses(tmp); len(alive) != 0 { + fmt.Fprintf(os.Stderr, "fake engines survived their tests: %+v\n", alive) + code = 1 + } + if remaining := sweepFakeProcesses(tmp); len(remaining) != 0 { + fmt.Fprintf(os.Stderr, "fake engine sweep failed: %+v\n", remaining) + code = 1 + } _ = os.RemoveAll(tmp) os.Exit(code) } diff --git a/services/nvpair-engine-manager/manager_test.go b/services/nvpair-engine-manager/manager_test.go index e5d7f08cc..e37ef59a2 100644 --- a/services/nvpair-engine-manager/manager_test.go +++ b/services/nvpair-engine-manager/manager_test.go @@ -69,7 +69,7 @@ func TestRunOpRejectsBadBind(t *testing.T) { func TestRunOpInstallAutostart(t *testing.T) { var out bytes.Buffer - ex := newTestExecutor(t, testEngineManifest(fakeEngineBin)) + ex := newTestExecutor(t, testEngineManifest(fakeEngineForTest(t))) t.Cleanup(func() { _ = ex.Stop("fake") }) m := NewManager(NewCodec(&out), ex, nil) allowReconciledTestStarts(t, m) @@ -94,7 +94,7 @@ func TestStatusQueriesDoNotBlockMessageDispatchDuringEngineOperation(t *testing. for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - ex := newTestExecutor(t, testEngineManifest(fakeEngineBin)) + ex := newTestExecutor(t, testEngineManifest(fakeEngineForTest(t))) st, err := ex.state("fake") if err != nil { t.Fatal(err) diff --git a/services/nvpair-engine-manager/model_delete_test.go b/services/nvpair-engine-manager/model_delete_test.go index e037eef48..79a8aa8e5 100644 --- a/services/nvpair-engine-manager/model_delete_test.go +++ b/services/nvpair-engine-manager/model_delete_test.go @@ -23,7 +23,7 @@ func TestLMStudioDeleteModelRemovePath(t *testing.T) { t.Fatal(err) } - m := testEngineManifest(fakeEngineBin) + m := testEngineManifest(fakeEngineForTest(t)) m.Actions["list_models"] = Action{ HTTP: &ActionHTTP{Method: "GET", Path: "/api/tags"}, Result: &ActionResult{Array: "models", Field: "name"}, @@ -71,14 +71,15 @@ func deleteModelRestartManifest(t *testing.T, root, bin string) *Manifest { // a test may delete or corrupt it without disturbing any other test. func privateEngineBin(t *testing.T) string { t.Helper() - data, err := os.ReadFile(fakeEngineBin) + data, err := os.ReadFile(fakeEngineForTest(t)) if err != nil { t.Fatalf("read fake engine: %v", err) } - bin := filepath.Join(t.TempDir(), "fake-engine"+filepath.Ext(fakeEngineBin)) + bin := filepath.Join(t.TempDir(), "fake-engine"+filepath.Ext(fakeEngineForTest(t))) if err := os.WriteFile(bin, data, 0o755); err != nil { t.Fatalf("write fake engine copy: %v", err) } + registerFakeExecutable(t, bin, fakeProcessDir(t)) return bin } @@ -106,7 +107,7 @@ func TestDeleteModelRestartAfterBouncesRunningEngine(t *testing.T) { t.Fatal(err) } - ex := newTestExecutor(t, deleteModelRestartManifest(t, root, fakeEngineBin)) + ex := newTestExecutor(t, deleteModelRestartManifest(t, root, fakeEngineForTest(t))) ctx := context.Background() t.Cleanup(func() { _ = ex.Stop("fake") }) if err := ex.Start(ctx, "fake"); err != nil { @@ -137,7 +138,7 @@ func TestDeleteModelRestartAfterLeavesStoppedEngineDown(t *testing.T) { t.Fatal(err) } - ex := newTestExecutor(t, deleteModelRestartManifest(t, root, fakeEngineBin)) + ex := newTestExecutor(t, deleteModelRestartManifest(t, root, fakeEngineForTest(t))) if _, err := ex.Action(context.Background(), "fake", "delete_model", []byte(`{"model":"`+modelRel+`"}`)); err != nil { t.Fatalf("delete_model: %v", err) } @@ -162,7 +163,7 @@ func TestDeleteModelWithoutRestartAfterDoesNotBounce(t *testing.T) { t.Fatal(err) } - m := deleteModelRestartManifest(t, root, fakeEngineBin) + m := deleteModelRestartManifest(t, root, fakeEngineForTest(t)) act := m.Actions["delete_model"] act.RestartAfter = false m.Actions["delete_model"] = act @@ -306,7 +307,7 @@ func hasRestartAfter(m *Manifest) bool { // restart_after makes ("the effect is visible when this replies") is only // keepable if doStart waits for a readiness probe. func TestRestartAfterRequiresReadinessProbe(t *testing.T) { - m := deleteModelRestartManifest(t, t.TempDir(), fakeEngineBin) + m := deleteModelRestartManifest(t, t.TempDir(), fakeEngineForTest(t)) for key, p := range m.Platforms { p.Runtime.Ready = nil m.Platforms[key] = p diff --git a/services/nvpair-engine-manager/model_test.go b/services/nvpair-engine-manager/model_test.go index 24d0cdbf6..cc9c6f4f0 100644 --- a/services/nvpair-engine-manager/model_test.go +++ b/services/nvpair-engine-manager/model_test.go @@ -20,7 +20,7 @@ import ( // are covered by TestCmdAction; LM Studio has no CLI model-delete (vendor // limitation), which is why only Ollama exposes delete_model. func TestModelLifecycle(t *testing.T) { - m := testEngineManifest(fakeEngineBin) // already has list_models (GET /api/tags) + m := testEngineManifest(fakeEngineForTest(t)) // already has list_models (GET /api/tags) m.Actions["pull_model"] = Action{HTTP: &ActionHTTP{Method: "POST", Path: "/api/pull"}} m.Actions["run_model"] = Action{HTTP: &ActionHTTP{Method: "POST", Path: "/api/generate"}} m.Actions["unload_model"] = Action{HTTP: &ActionHTTP{Method: "POST", Path: "/api/generate"}} diff --git a/services/nvpair-engine-manager/models_test.go b/services/nvpair-engine-manager/models_test.go index e37f09d0b..a4a3e181e 100644 --- a/services/nvpair-engine-manager/models_test.go +++ b/services/nvpair-engine-manager/models_test.go @@ -132,7 +132,7 @@ func TestExtractStringsResultDistinguishesEmptyFromUnknown(t *testing.T) { // with a Result-bearing list_models action contributes its models; a stopped // engine contributes nothing. func TestModels(t *testing.T) { - m := testEngineManifest(fakeEngineBin) + m := testEngineManifest(fakeEngineForTest(t)) m.Actions["list_models"] = Action{ HTTP: &ActionHTTP{Method: "GET", Path: "/api/tags"}, Result: &ActionResult{Array: "models", Field: "name"}, @@ -167,7 +167,7 @@ func TestModels(t *testing.T) { // flat union and the per-engine attribution are both derived from one sweep, // and a stopped engine yields neither. func TestModelsResult(t *testing.T) { - m := testEngineManifest(fakeEngineBin) + m := testEngineManifest(fakeEngineForTest(t)) m.Actions["list_models"] = Action{ HTTP: &ActionHTTP{Method: "GET", Path: "/api/tags"}, Result: &ActionResult{Array: "models", Field: "name"}, @@ -216,8 +216,8 @@ func TestModelsResult(t *testing.T) { // loadedActionManifest is testEngineManifest with both list_models (/api/tags) // and loaded_models (/api/ps) actions declared, so the executor surfaces // LoadedByEngine from the fake engine's resident set. -func loadedActionManifest() *Manifest { - m := testEngineManifest(fakeEngineBin) +func loadedActionManifest(t *testing.T) *Manifest { + m := testEngineManifest(fakeEngineForTest(t)) m.Actions["list_models"] = Action{ HTTP: &ActionHTTP{Method: "GET", Path: "/api/tags"}, Result: &ActionResult{Array: "models", Field: "name"}, @@ -251,7 +251,7 @@ func setLoaded(t *testing.T, ex *Executor, names []string) { // eviction leaves the engine key present with an empty list ("running, nothing // loaded") while the installed list is unchanged. func TestModelsResultLoaded(t *testing.T) { - ex := newTestExecutor(t, loadedActionManifest()) + ex := newTestExecutor(t, loadedActionManifest(t)) ctx := context.Background() t.Cleanup(func() { _ = ex.Stop("fake") }) @@ -278,7 +278,7 @@ func TestModelsResultLoaded(t *testing.T) { // action contributes no LoadedByEngine key (the map stays nil), so the field is // omitted on the wire for engines that don't support loaded reporting. func TestModelsResultNoLoadedActionOmitsKey(t *testing.T) { - m := testEngineManifest(fakeEngineBin) + m := testEngineManifest(fakeEngineForTest(t)) m.Actions["list_models"] = Action{ HTTP: &ActionHTTP{Method: "GET", Path: "/api/tags"}, Result: &ActionResult{Array: "models", Field: "name"}, @@ -301,7 +301,7 @@ func TestModelsResultNoLoadedActionOmitsKey(t *testing.T) { // change, and a sweep after an eviction reports the engine as changed and // carries the fresh full model result as the push payload. func TestSweepLoadedSeedsThenEmitsOnChange(t *testing.T) { - ex := newTestExecutor(t, loadedActionManifest()) + ex := newTestExecutor(t, loadedActionManifest(t)) ctx := context.Background() t.Cleanup(func() { _ = ex.Stop("fake") }) if err := ex.Start(ctx, "fake"); err != nil { @@ -336,7 +336,7 @@ func TestSweepLoadedSeedsThenEmitsOnChange(t *testing.T) { // from a stop) is NOT reported as changed and keeps its last-good baseline, so a // single blip can't emit a spurious drop-then-re-add pair. func TestSweepLoadedRetainsLastGoodOnTransientMiss(t *testing.T) { - ex := newTestExecutor(t, loadedActionManifest()) + ex := newTestExecutor(t, loadedActionManifest(t)) prev := map[string][]string{"fake": {"llama3.2:1b"}} // The engine isn't started, so ModelsResult reports it neither running nor // queryable: LoadedByEngine has no "fake" key this sweep. diff --git a/services/nvpair-engine-manager/process_cleanup_test.go b/services/nvpair-engine-manager/process_cleanup_test.go new file mode 100644 index 000000000..f3acc4096 --- /dev/null +++ b/services/nvpair-engine-manager/process_cleanup_test.go @@ -0,0 +1,94 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "context" + "encoding/json" + "os" + "os/exec" + "path/filepath" + "strconv" + "strings" + "testing" + "time" +) + +func TestFakeEngineCleanupOnFailedTest(t *testing.T) { testFakeEngineFailure(t, false) } +func TestFakeEngineTestMainSweepsMissedCleanup(t *testing.T) { testFakeEngineFailure(t, true) } + +func testFakeEngineFailure(t *testing.T, sweepOnly bool) { + t.Helper() + marker := filepath.Join(t.TempDir(), "pid") + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + cmd := exec.CommandContext(ctx, os.Args[0], "-test.run=^TestFakeEngineFailureFixture$") + cmd.WaitDelay = 2 * time.Second + cmd.Env = append(os.Environ(), "PAIR_TEST_FAILURE_MARKER="+marker, "PAIR_TEST_SWEEP_ONLY="+strconv.FormatBool(sweepOnly)) + out, err := cmd.CombinedOutput() + if err == nil || !strings.Contains(string(out), "intentional failure after spawn") { + t.Fatalf("fixture did not fail as intended: %v\n%s", err, out) + } + if sweepOnly && !strings.Contains(string(out), "test process survived") { + t.Fatalf("TestMain failed to detect the missed cleanup: %s", out) + } + data, err := os.ReadFile(marker) + if err != nil { + t.Fatal(err) + } + var record fakeProcessRecord + if err := json.Unmarshal(data, &record); err != nil { + t.Fatal(err) + } + pid := record.PID + t.Cleanup(func() { + if isOurEngineImage(testProcessImage(pid), record.Image) { + killTestProcessGroup(pid) + } + }) + deadline := time.Now().Add(2 * time.Second) + for pidAlive(pid) && testProcessImage(pid) != "" && time.Now().Before(deadline) { + time.Sleep(10 * time.Millisecond) + } + if pidAlive(pid) && testProcessImage(pid) != "" { + t.Fatalf("fake engine pid %d survived its failed test and test binary exit", pid) + } +} + +func TestFakeEngineFailureFixture(t *testing.T) { + marker := os.Getenv("PAIR_TEST_FAILURE_MARKER") + if marker == "" { + return + } + var ex *Executor + if os.Getenv("PAIR_TEST_SWEEP_ONLY") == "true" { + // Intentionally bypass per-test cleanup to prove TestMain catches it. + owner, err := os.MkdirTemp(testProcessRoot, "missed-") + if err != nil { + t.Fatal(err) + } + registerFakeExecutable(t, fakeEngineBin, owner) + reg := NewRegistry() + m := testEngineManifest(fakeEngineBin) + reg.engines[m.Engine] = m + ex = reconciledTestExecutor(NewExecutor(reg, NewReporter(nil), nil, t.TempDir())) + } else { + ex = newTestExecutor(t, testEngineManifest(fakeEngineForTest(t))) + } + if err := ex.Start(context.Background(), "fake"); err != nil { + t.Fatal(err) + } + st, _ := ex.state("fake") + st.mu.Lock() + record := fakeProcessRecord{PID: st.proc.cmd.Process.Pid, Image: st.proc.cmd.Path} + st.mu.Unlock() + data, err := json.Marshal(record) + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(marker, data, 0600); err != nil { + t.Fatal(err) + } + t.Fatal("intentional failure after spawn") +} diff --git a/services/nvpair-engine-manager/progress_test.go b/services/nvpair-engine-manager/progress_test.go index 4e289d038..d8ce60d48 100644 --- a/services/nvpair-engine-manager/progress_test.go +++ b/services/nvpair-engine-manager/progress_test.go @@ -146,10 +146,10 @@ func TestInstallFlowReportsOnlyMeasurablePercents(t *testing.T) { Detect: []string{bin}, Install: &Install{ Fetch: &Fetch{URL: srv.URL, SHA256: hex.EncodeToString(sum[:])}, - Run: []string{fakeEngineBin, "touch", bin}, + Run: []string{fakeEngineForTest(t), "touch", bin}, Mode: "user", }, - Runtime: Runtime{Bin: fakeEngineBin}, + Runtime: Runtime{Bin: fakeEngineForTest(t)}, }}, } diff --git a/services/nvpair-engine-manager/pull_failure_test.go b/services/nvpair-engine-manager/pull_failure_test.go index 8fc5fd9f7..9a6533505 100644 --- a/services/nvpair-engine-manager/pull_failure_test.go +++ b/services/nvpair-engine-manager/pull_failure_test.go @@ -28,7 +28,7 @@ func TestPullEngineErrorBecomesRPCError(t *testing.T) { _, _ = w.Write([]byte("{\"status\":\"pulling manifest\"}\n{\"error\":\"model does not exist\"}\n")) })) defer engine.Close() - manifest := testEngineManifest(fakeEngineBin) + manifest := testEngineManifest(fakeEngineForTest(t)) manifest.Actions["pull_model"] = Action{HTTP: &ActionHTTP{Method: "POST", Path: "/api/pull"}} ex := newTestExecutor(t, manifest) st, err := ex.state("fake") diff --git a/services/nvpair-engine-manager/pull_test.go b/services/nvpair-engine-manager/pull_test.go index 11488448d..38f2a60e0 100644 --- a/services/nvpair-engine-manager/pull_test.go +++ b/services/nvpair-engine-manager/pull_test.go @@ -65,7 +65,7 @@ func TestModelFromParams(t *testing.T) { // still returns the pull's terminal result — matching what remote pulls already // surface via engine:remote-progress. func TestActionPullModelStreamsProgress(t *testing.T) { - m := testEngineManifest(fakeEngineBin) + m := testEngineManifest(fakeEngineForTest(t)) m.Actions["pull_model"] = Action{HTTP: &ActionHTTP{Method: "POST", Path: "/api/pull"}} var mu sync.Mutex @@ -123,8 +123,8 @@ func TestActionPullModelStreamsProgress(t *testing.T) { // returns the command's terminal result — the counterpart to the HTTP streaming // path in TestActionPullModelStreamsProgress. func TestActionPullModelCmdMarkerAndResult(t *testing.T) { - m := testEngineManifest(fakeEngineBin) - m.Actions["pull_model"] = Action{Cmd: []string{fakeEngineBin, "echo", "pulled:{model}"}} + m := testEngineManifest(fakeEngineForTest(t)) + m.Actions["pull_model"] = Action{Cmd: []string{fakeEngineForTest(t), "echo", "pulled:{model}"}} var mu sync.Mutex var pulls []map[string]any @@ -169,7 +169,7 @@ func TestActionPullModelCmdMarkerAndResult(t *testing.T) { // error), so a UI that already stopped waiting on the synchronous call still // converges off "pulling" instead of appearing stuck. func TestActionPullModelFailureEmitsTerminalError(t *testing.T) { - m := testEngineManifest(fakeEngineBin) + m := testEngineManifest(fakeEngineForTest(t)) m.Actions["pull_model"] = Action{HTTP: &ActionHTTP{Method: "POST", Path: "/api/pull"}} var mu sync.Mutex diff --git a/services/nvpair-engine-manager/remediation_test.go b/services/nvpair-engine-manager/remediation_test.go index dd9ba3bc9..2ed8efc7f 100644 --- a/services/nvpair-engine-manager/remediation_test.go +++ b/services/nvpair-engine-manager/remediation_test.go @@ -64,7 +64,7 @@ func TestDownloadUnpinned(t *testing.T) { _, _ = w.Write(payload) })) defer srv.Close() - ex := newTestExecutor(t, testEngineManifest(fakeEngineBin)) + ex := newTestExecutor(t, testEngineManifest(fakeEngineForTest(t))) p, err := ex.download(context.Background(), "fake", &Fetch{URL: srv.URL}) // no SHA256 if err != nil { t.Fatalf("unpinned download should succeed, got %v", err) @@ -104,7 +104,7 @@ func TestUninstallRetries(t *testing.T) { dir := t.TempDir() binCopy := filepath.Join(dir, "fake"+exeExt()) - copyFile(t, fakeEngineBin, binCopy) + copyFile(t, fakeEngineForTest(t), binCopy) marker := filepath.Join(dir, "attempts") m := testEngineManifest(binCopy) @@ -171,15 +171,15 @@ func TestActionReservedPlaceholderWins(t *testing.T) { m := &Manifest{ Engine: "fake", DisplayName: "Fake", ManifestVersion: 1, Platforms: map[string]Platform{hostKey(): { - Detect: []string{fakeEngineBin}, - Runtime: Runtime{CLI: fakeEngineBin, Bin: fakeEngineBin}, + Detect: []string{fakeEngineForTest(t)}, + Runtime: Runtime{CLI: fakeEngineForTest(t), Bin: fakeEngineForTest(t)}, }}, Actions: map[string]Action{"echo": {Cmd: []string{"{cli}", "echo", "{model}"}}}, } ex, _ := capturingExecutor(t, m) // Malicious params try to override {cli} (argv[0]). Reserved must win, - // so the command runs fakeEngineBin (not the bogus path) and echoes + // so the command runs fakeEngineForTest(t) (not the bogus path) and echoes // the model value. res, err := ex.Action(context.Background(), "fake", "echo", json.RawMessage(`{"cli":"definitely-not-a-real-binary","model":"safevalue"}`)) @@ -197,9 +197,9 @@ func TestReadinessTimeoutNoSpuriousExit(t *testing.T) { m := &Manifest{ Engine: "slow", DisplayName: "Slow", ManifestVersion: 1, Platforms: map[string]Platform{hostKey(): { - Detect: []string{fakeEngineBin}, + Detect: []string{fakeEngineForTest(t)}, Runtime: Runtime{ - Bin: fakeEngineBin, + Bin: fakeEngineForTest(t), Args: []string{"noserve"}, // runs but never binds Ready: &Probe{TCP: "127.0.0.1:{port}", TimeoutS: 1}, }, @@ -234,7 +234,7 @@ func TestReadinessTimeoutNoSpuriousExit(t *testing.T) { // for engines whose initialization finishes after several failed probes. Start // must keep the owned process alive until the configured finite budget expires. func TestStartWaitsForDelayedReadinessWithinBudget(t *testing.T) { - m := testEngineManifest(fakeEngineBin) + m := testEngineManifest(fakeEngineForTest(t)) p := m.Platforms[hostKey()] p.Runtime.Env["FAKE_START_DELAY"] = "750ms" p.Runtime.Ready.TimeoutS = 5 @@ -263,7 +263,7 @@ func TestStartWaitsForDelayedReadinessWithinBudget(t *testing.T) { // TestConcurrentStartIsSerialized guards the per-engine op lock: many // concurrent Starts must not race or double-spawn. Run under -race. func TestConcurrentStartIsSerialized(t *testing.T) { - ex, _ := capturingExecutor(t, testEngineManifest(fakeEngineBin)) + ex, _ := capturingExecutor(t, testEngineManifest(fakeEngineForTest(t))) t.Cleanup(func() { _ = ex.Stop("fake") }) var wg sync.WaitGroup for i := 0; i < 8; i++ { @@ -285,8 +285,8 @@ func TestInstallChecksumMismatchReported(t *testing.T) { Engine: "fake", DisplayName: "Fake", ManifestVersion: 1, Platforms: map[string]Platform{hostKey(): { Detect: []string{filepath.Join(t.TempDir(), "never"+exeExt())}, // stays not-installed - Install: &Install{Fetch: &Fetch{URL: srv.URL, SHA256: "deadbeef"}, Run: []string{fakeEngineBin, "echo", "x"}, Mode: "user"}, - Runtime: Runtime{Bin: fakeEngineBin}, + Install: &Install{Fetch: &Fetch{URL: srv.URL, SHA256: "deadbeef"}, Run: []string{fakeEngineForTest(t), "echo", "x"}, Mode: "user"}, + Runtime: Runtime{Bin: fakeEngineForTest(t)}, }}, } ex, c := capturingExecutor(t, m) @@ -318,12 +318,12 @@ func TestHealthFlipReportsAndClears(t *testing.T) { m := &Manifest{ Engine: "d", DisplayName: "Daemon", ManifestVersion: 1, Platforms: map[string]Platform{hostKey(): { - Detect: []string{fakeEngineBin}, + Detect: []string{fakeEngineForTest(t)}, Runtime: Runtime{ Mode: "command", Port: port, - Start: [][]string{{fakeEngineBin, "echo", "up"}}, - Stop: &StopSpec{Cmd: []string{fakeEngineBin, "echo", "down"}}, + Start: [][]string{{fakeEngineForTest(t), "echo", "up"}}, + Stop: &StopSpec{Cmd: []string{fakeEngineForTest(t), "echo", "down"}}, Ready: &Probe{HTTP: "http://127.0.0.1:{port}/", Status: 200, TimeoutS: 5}, Health: &Probe{HTTP: "http://127.0.0.1:{port}/", Status: 200, IntervalS: 1}, }, @@ -342,7 +342,7 @@ func TestHealthFlipReportsAndClears(t *testing.T) { } func TestUnexpectedExitReported(t *testing.T) { - ex, _ := capturingExecutor(t, testEngineManifest(fakeEngineBin)) + ex, _ := capturingExecutor(t, testEngineManifest(fakeEngineForTest(t))) if err := ex.Start(context.Background(), "fake"); err != nil { t.Fatalf("start: %v", err) } @@ -358,7 +358,7 @@ func TestUnexpectedExitReported(t *testing.T) { } func TestNormalStopIsSilent(t *testing.T) { - ex, _ := capturingExecutor(t, testEngineManifest(fakeEngineBin)) + ex, _ := capturingExecutor(t, testEngineManifest(fakeEngineForTest(t))) if err := ex.Start(context.Background(), "fake"); err != nil { t.Fatalf("start: %v", err) } @@ -372,7 +372,7 @@ func TestNormalStopIsSilent(t *testing.T) { } func TestRestartProcess(t *testing.T) { - ex, _ := capturingExecutor(t, testEngineManifest(fakeEngineBin)) + ex, _ := capturingExecutor(t, testEngineManifest(fakeEngineForTest(t))) t.Cleanup(func() { _ = ex.Stop("fake") }) if err := ex.Start(context.Background(), "fake"); err != nil { t.Fatalf("start: %v", err) @@ -386,7 +386,7 @@ func TestRestartProcess(t *testing.T) { } func TestActionUnknownAndHTTPError(t *testing.T) { - m := testEngineManifest(fakeEngineBin) + m := testEngineManifest(fakeEngineForTest(t)) m.Actions["err"] = Action{HTTP: &ActionHTTP{Method: "GET", Path: "/api/error"}} ex, _ := capturingExecutor(t, m) t.Cleanup(func() { _ = ex.Stop("fake") }) @@ -482,7 +482,7 @@ func TestDownloadSizeCap(t *testing.T) { })) defer srv.Close() - ex, _ := capturingExecutor(t, testEngineManifest(fakeEngineBin)) + ex, _ := capturingExecutor(t, testEngineManifest(fakeEngineForTest(t))) if _, err := ex.download(context.Background(), "x", &Fetch{URL: srv.URL, SHA256: "abc"}); err == nil || !strings.Contains(err.Error(), "exceeds") { t.Fatalf("expected size-cap error, got %v", err) } @@ -507,12 +507,12 @@ func TestStopClearsUnhealthy(t *testing.T) { m := &Manifest{ Engine: "d", DisplayName: "Daemon", ManifestVersion: 1, Platforms: map[string]Platform{hostKey(): { - Detect: []string{fakeEngineBin}, + Detect: []string{fakeEngineForTest(t)}, Runtime: Runtime{ Mode: "command", Port: port, - Start: [][]string{{fakeEngineBin, "echo", "up"}}, - Stop: &StopSpec{Cmd: []string{fakeEngineBin, "touch", marker}, GraceS: 2}, + Start: [][]string{{fakeEngineForTest(t), "echo", "up"}}, + Stop: &StopSpec{Cmd: []string{fakeEngineForTest(t), "touch", marker}, GraceS: 2}, Ready: &Probe{HTTP: "http://127.0.0.1:{port}/", Status: 200, TimeoutS: 5}, Health: &Probe{HTTP: "http://127.0.0.1:{port}/", Status: 200, IntervalS: 1}, }, diff --git a/services/nvpair-engine-manager/reservedport_test.go b/services/nvpair-engine-manager/reservedport_test.go index ad68959b2..4427fe8bd 100644 --- a/services/nvpair-engine-manager/reservedport_test.go +++ b/services/nvpair-engine-manager/reservedport_test.go @@ -12,7 +12,7 @@ import ( ) func TestReservedAliasPortBlocksLocalAndRemoteStarts(t *testing.T) { - manifest := testEngineManifest(fakeEngineBin) + manifest := testEngineManifest(fakeEngineForTest(t)) reg := NewRegistry() reg.engines[manifest.Engine] = manifest exec := reconciledTestExecutor(NewExecutor(reg, NewReporter(nil), func(string, any) {}, t.TempDir())) diff --git a/services/nvpair-engine-manager/setport_test.go b/services/nvpair-engine-manager/setport_test.go index 0a7c0e28c..3c8b05d59 100644 --- a/services/nvpair-engine-manager/setport_test.go +++ b/services/nvpair-engine-manager/setport_test.go @@ -53,7 +53,7 @@ func newBundledExecutor(t *testing.T, overrideDir string) *Executor { func adoptedEngineFixture(t *testing.T, engine string, port int, rt Runtime) *Executor { t.Helper() - m := testEngineManifest(fakeEngineBin) + m := testEngineManifest(fakeEngineForTest(t)) m.Engine = engine m.DisplayName = engine for key, platform := range m.Platforms { @@ -84,8 +84,8 @@ func adoptedCommandEngineFixture(t *testing.T, engine string, port int) (*Execut ex := adoptedEngineFixture(t, engine, port, Runtime{ Mode: "command", Port: port, - Start: [][]string{{fakeEngineBin, "touch", startMarker}}, - Stop: &StopSpec{Cmd: []string{fakeEngineBin, "touch", stopMarker}}, + Start: [][]string{{fakeEngineForTest(t), "touch", startMarker}}, + Stop: &StopSpec{Cmd: []string{fakeEngineForTest(t), "touch", stopMarker}}, }) return ex, func() bool { return fileExists(filepath.Join(dir, "stopped-"+strconv.Itoa(port))) }, @@ -95,7 +95,7 @@ func adoptedCommandEngineFixture(t *testing.T, engine string, port int) (*Execut func adoptedProcessEngineFixture(t *testing.T, engine string, port int) *Executor { t.Helper() return adoptedEngineFixture(t, engine, port, Runtime{ - Bin: fakeEngineBin, + Bin: fakeEngineForTest(t), Port: port, Stop: &StopSpec{Signal: "term", GraceS: 3}, }) @@ -106,7 +106,7 @@ func adoptedCommandEngineWithoutStopFixture(t *testing.T, engine string, port in return adoptedEngineFixture(t, engine, port, Runtime{ Mode: "command", Port: port, - Start: [][]string{{fakeEngineBin, "echo", "up"}}, + Start: [][]string{{fakeEngineForTest(t), "echo", "up"}}, }) } diff --git a/services/nvpair-engine-manager/settings_security_test.go b/services/nvpair-engine-manager/settings_security_test.go index fa9f7857c..e68e76c67 100644 --- a/services/nvpair-engine-manager/settings_security_test.go +++ b/services/nvpair-engine-manager/settings_security_test.go @@ -37,10 +37,10 @@ func TestManagedSettingsRejectBeforeAnyRuntimeMutation(t *testing.T) { } func TestOMLXOnlyProxySettingsForRunningApp(t *testing.T) { - manifest := testEngineManifest(fakeEngineBin) + manifest := testEngineManifest(fakeEngineForTest(t)) manifest.Engine = "omlx" for key, platform := range manifest.Platforms { - platform.Runtime = Runtime{Mode: "command", CLI: fakeEngineBin, Port: 1235, Start: [][]string{{"{cli}", "start"}}} + platform.Runtime = Runtime{Mode: "command", CLI: fakeEngineForTest(t), Port: 1235, Start: [][]string{{"{cli}", "start"}}} manifest.Platforms[key] = platform } e := newTestExecutor(t, manifest) diff --git a/services/nvpair-engine-manager/settings_test.go b/services/nvpair-engine-manager/settings_test.go index 9eda64684..228ce30a2 100644 --- a/services/nvpair-engine-manager/settings_test.go +++ b/services/nvpair-engine-manager/settings_test.go @@ -23,7 +23,7 @@ import ( func settingsExecutor(t *testing.T, command bool) *Executor { t.Helper() - m := testEngineManifest(fakeEngineBin) + m := testEngineManifest(fakeEngineForTest(t)) key := runtime.GOOS + "/" + runtime.GOARCH p := m.Platforms[key] port, err := freePort() @@ -40,7 +40,7 @@ func settingsExecutor(t *testing.T, command bool) *Executor { } if command { p.Runtime.Mode = "command" - p.Runtime.CLI = fakeEngineBin + p.Runtime.CLI = fakeEngineForTest(t) p.Runtime.Start = [][]string{{"{cli}", "server", "start", "--port", "{port}", "--bind", "{host}"}} p.Runtime.EditableLaunch = &EditableLaunch{ FixedArgs: []string{"server", "start"}, @@ -208,7 +208,7 @@ func TestSettingsPreviewAcceptsUnrelatedOptions(t *testing.T) { func TestSettingsArgumentsDoNotIncludeExecutableOrSubcommand(t *testing.T) { eachEngineMode(t, func(t *testing.T, e *Executor) { p := settingsRequest(t, e) - if strings.Contains(p.Settings.LaunchText, fakeEngineBin) || strings.Contains(p.Settings.LaunchText, "server start") || strings.Contains(p.Settings.LaunchText, "serve") { + if strings.Contains(p.Settings.LaunchText, fakeEngineForTest(t)) || strings.Contains(p.Settings.LaunchText, "server start") || strings.Contains(p.Settings.LaunchText, "serve") { t.Fatalf("manifest command leaked into editor: %q", p.Settings.LaunchText) } p.Settings.LaunchText = "different serve" @@ -225,7 +225,7 @@ func TestSettingsArgumentsDoNotIncludeExecutableOrSubcommand(t *testing.T) { t.Fatal(err) } actual, err := applyLiteralLaunch(rt, launch, map[string]string{"host": "127.0.0.1", "port": fmt.Sprint(p.Settings.ServerPort)}) - if err != nil || actual.Bin != fakeEngineBin || !slices.Equal(actual.Args[:len(rt.EditableLaunch.FixedArgs)], rt.EditableLaunch.FixedArgs) { + if err != nil || actual.Bin != fakeEngineForTest(t) || !slices.Equal(actual.Args[:len(rt.EditableLaunch.FixedArgs)], rt.EditableLaunch.FixedArgs) { t.Fatalf("arguments changed the executable/subcommand: %+v %v", actual, err) } }) @@ -643,11 +643,11 @@ func TestSettingsCommandLaunchPreservesLiteralsAndCleansFailedStart(t *testing.T capture := filepath.Join(t.TempDir(), "argv.json") stopped := filepath.Join(t.TempDir(), "stopped") rt := &st.plat.Runtime - rt.Start = [][]string{{fakeEngineBin, "captureargs", capture, "--port", "{port}", "--bind", "{host}"}} + rt.Start = [][]string{{fakeEngineForTest(t), "captureargs", capture, "--port", "{port}", "--bind", "{host}"}} rt.EditableLaunch.FixedArgs = []string{"captureargs", capture} rt.Ready = nil rt.Health = nil - rt.Stop = &StopSpec{Cmd: []string{fakeEngineBin, "touch", stopped}} + rt.Stop = &StopSpec{Cmd: []string{fakeEngineForTest(t), "touch", stopped}} p := settingsRequest(t, e) p.Settings.LaunchText += ` "{port}" "$HOME" "two words" ""` if _, err := e.ConfigureLaunch(context.Background(), settings.Configure{Engine: "fake", Settings: p.Settings}, func(context.Context, bool) error { return nil }); err != nil { @@ -689,10 +689,10 @@ func TestSettingsEnvironmentReachesEngineAndCanBeEditedAndRemoved(t *testing.T) if command { capture := filepath.Join(t.TempDir(), "args.json") rt := &st.plat.Runtime - rt.Start = [][]string{{fakeEngineBin, "captureargs", capture, "--port", "{port}", "--bind", "{host}"}} + rt.Start = [][]string{{fakeEngineForTest(t), "captureargs", capture, "--port", "{port}", "--bind", "{host}"}} rt.EditableLaunch.FixedArgs = []string{"captureargs", capture} rt.Ready, rt.Health = nil, nil - rt.Stop = &StopSpec{Cmd: []string{fakeEngineBin, "touch", filepath.Join(t.TempDir(), "stopped")}} + rt.Stop = &StopSpec{Cmd: []string{fakeEngineForTest(t), "touch", filepath.Join(t.TempDir(), "stopped")}} } captureEnv := filepath.Join(t.TempDir(), "env.json") prefix, err := formatLaunchText([]string{"PAIR_TEST_ENV_FILE=" + captureEnv, "PAIR_TEST_CUSTOM=first-value", `PAIR_TEST_LITERAL=$HOME {port} C:\new\tools`, "PAIR_TEST_EMPTY="}) @@ -774,10 +774,10 @@ func TestSettingsFailedStopLeavesSavedConfigurationIntact(t *testing.T) { st := settingsState(t, e) capture := filepath.Join(t.TempDir(), "argv.json") rt := &st.plat.Runtime - rt.Start = [][]string{{fakeEngineBin, "captureargs", capture, "--port", "{port}", "--bind", "{host}"}} + rt.Start = [][]string{{fakeEngineForTest(t), "captureargs", capture, "--port", "{port}", "--bind", "{host}"}} rt.EditableLaunch.FixedArgs = []string{"captureargs", capture} rt.Ready, rt.Health = nil, nil - rt.Stop = &StopSpec{Cmd: []string{fakeEngineBin, "failmark", filepath.Join(t.TempDir(), "stop-attempts")}} + rt.Stop = &StopSpec{Cmd: []string{fakeEngineForTest(t), "failmark", filepath.Join(t.TempDir(), "stop-attempts")}} if err := e.Start(context.Background(), "fake"); err != nil { t.Fatal(err) } @@ -861,7 +861,7 @@ func TestSettingsOmlxLaunchStateAndRestrictions(t *testing.T) { Platforms: map[string]Platform{ runtime.GOOS + "/" + runtime.GOARCH: { Runtime: Runtime{ - Bin: fakeEngineBin, + Bin: fakeEngineForTest(t), Port: 1235, Args: []string{"serve"}, }, diff --git a/services/nvpair-engine-manager/shutdown_once_test.go b/services/nvpair-engine-manager/shutdown_once_test.go index 7b10d2ab8..e954fba78 100644 --- a/services/nvpair-engine-manager/shutdown_once_test.go +++ b/services/nvpair-engine-manager/shutdown_once_test.go @@ -49,7 +49,7 @@ func adoptedEngineOnLivePort(t *testing.T, ex *Executor) { // engine whose stop can only be declined stays running, so each repeat sweep // would re-pay doStop's readiness probe for it. func TestStopAll_RunsOneSweepPerProcess(t *testing.T) { - ex := newTestExecutor(t, testEngineManifest(fakeEngineBin)) + ex := newTestExecutor(t, testEngineManifest(fakeEngineForTest(t))) adoptedEngineOnLivePort(t, ex) ex.StopAll() @@ -76,7 +76,7 @@ func TestStopAll_RunsOneSweepPerProcess(t *testing.T) { // the broker would close stdin, engine-manager would exit, and the engines it // launched would be left running with nothing owning them. func TestStopAll_WaitsForTheSweepInFlight(t *testing.T) { - ex := newTestExecutor(t, testEngineManifest(fakeEngineBin)) + ex := newTestExecutor(t, testEngineManifest(fakeEngineForTest(t))) adoptedEngineOnLivePort(t, ex) st, err := ex.state("fake") require.NoError(t, err) @@ -171,12 +171,12 @@ func TestStopAll_RetriesAFailedStopCommand(t *testing.T) { Engine: "daemon", DisplayName: "Daemon Engine", ManifestVersion: 1, Platforms: map[string]Platform{ hostKey(): { - Detect: []string{fakeEngineBin}, + Detect: []string{fakeEngineForTest(t)}, Runtime: Runtime{ Mode: "command", Port: port, - Start: [][]string{{fakeEngineBin, "touch", startMarker}}, - Stop: &StopSpec{Cmd: []string{fakeEngineBin, "failoncethentouch", stopAttempt, stopMarker}}, + Start: [][]string{{fakeEngineForTest(t), "touch", startMarker}}, + Stop: &StopSpec{Cmd: []string{fakeEngineForTest(t), "failoncethentouch", stopAttempt, stopMarker}}, Ready: &Probe{HTTP: "http://127.0.0.1:{port}/", Status: http.StatusOK, TimeoutS: 10}, }, }, diff --git a/services/nvpair-engine-manager/shutdown_tail_test.go b/services/nvpair-engine-manager/shutdown_tail_test.go index 620b05691..c4a06325f 100644 --- a/services/nvpair-engine-manager/shutdown_tail_test.go +++ b/services/nvpair-engine-manager/shutdown_tail_test.go @@ -27,7 +27,7 @@ func testCommandStopTail(t *testing.T, delay string, grace int, wantTimeout bool if err != nil { t.Fatal(err) } - cmd := exec.Command(fakeEngineBin, "shutdown-tail", marker, "127.0.0.1:"+strconv.Itoa(port), delay) + cmd := exec.Command(fakeEngineForTest(t), "shutdown-tail", marker, "127.0.0.1:"+strconv.Itoa(port), delay) configureSysProcAttr(cmd) if err := cmd.Start(); err != nil { t.Fatal(err) @@ -39,11 +39,11 @@ func testCommandStopTail(t *testing.T, delay string, grace int, wantTimeout bool if pid, _, ok := pidOnPort(port); !ok || pid != cmd.Process.Pid { t.Skip("host cannot resolve listener PID") } - m := testEngineManifest(fakeEngineBin) + m := testEngineManifest(fakeEngineForTest(t)) p := m.Platforms[hostKey()] p.Runtime.Mode = "command" p.Runtime.Port = port - p.Runtime.Stop = &StopSpec{Cmd: []string{fakeEngineBin, "touch", marker}, GraceS: grace} + p.Runtime.Stop = &StopSpec{Cmd: []string{fakeEngineForTest(t), "touch", marker}, GraceS: grace} p.Runtime.Health = nil m.Platforms[hostKey()] = p ex := newTestExecutor(t, m) diff --git a/services/nvpair-engine-manager/shutdown_test.go b/services/nvpair-engine-manager/shutdown_test.go index 291ab72f1..2e215dc53 100644 --- a/services/nvpair-engine-manager/shutdown_test.go +++ b/services/nvpair-engine-manager/shutdown_test.go @@ -25,7 +25,7 @@ func TestStopAllStopsEveryEngine(t *testing.T) { reg := NewRegistry() names := []string{"fake-a", "fake-b"} for _, name := range names { - m := testEngineManifest(fakeEngineBin) + m := testEngineManifest(fakeEngineForTest(t)) m.Engine = name reg.engines[name] = m } @@ -60,11 +60,11 @@ func TestStopAllCancelsCommandStartWithoutError(t *testing.T) { pidFile := filepath.Join(t.TempDir(), "fake.pid") t.Setenv("FAKE_PID_FILE", pidFile) - manifest := testEngineManifest(fakeEngineBin) + manifest := testEngineManifest(fakeEngineForTest(t)) platform := manifest.Platforms[hostKey()] platform.Runtime.Mode = "command" platform.Runtime.Bin = "" - platform.Runtime.Start = [][]string{{fakeEngineBin}} + platform.Runtime.Start = [][]string{{fakeEngineForTest(t)}} platform.Runtime.Ready = nil platform.Runtime.Health = nil platform.Runtime.Stop = nil @@ -127,13 +127,13 @@ func testStopAllDetachedCommandDaemon(t *testing.T, command string) { t.Fatalf("free port: %v", err) } - manifest := testEngineManifest(fakeEngineBin) + manifest := testEngineManifest(fakeEngineForTest(t)) platform := manifest.Platforms[hostKey()] platform.Runtime.Mode = "command" platform.Runtime.Bin = "" platform.Runtime.Port = port - platform.Runtime.Start = [][]string{{fakeEngineBin, command, startMarker}} - platform.Runtime.Stop = &StopSpec{Cmd: []string{fakeEngineBin, "touch", stopMarker}, GraceS: 1} + platform.Runtime.Start = [][]string{{fakeEngineForTest(t), command, startMarker}} + platform.Runtime.Stop = &StopSpec{Cmd: []string{fakeEngineForTest(t), "touch", stopMarker}, GraceS: 1} platform.Runtime.Ready = &Probe{ HTTP: "http://127.0.0.1:{port}/", Status: http.StatusOK, @@ -228,12 +228,12 @@ func testStopAllDetachedCommandDaemon(t *testing.T, command string) { func TestFailedCommandSpawnDoesNotRunStopCLI(t *testing.T) { marker := filepath.Join(t.TempDir(), "stopped") - manifest := testEngineManifest(fakeEngineBin) + manifest := testEngineManifest(fakeEngineForTest(t)) p := manifest.Platforms[hostKey()] p.Runtime.Mode = "command" p.Runtime.Bin = "" p.Runtime.Start = [][]string{{filepath.Join(t.TempDir(), "nonexistent")}} - p.Runtime.Stop = &StopSpec{Cmd: []string{fakeEngineBin, "touch", marker}} + p.Runtime.Stop = &StopSpec{Cmd: []string{fakeEngineForTest(t), "touch", marker}} p.Runtime.Ready, p.Runtime.Health = nil, nil manifest.Platforms[hostKey()] = p ex := newTestExecutor(t, manifest) diff --git a/services/nvpair-engine-manager/status_test.go b/services/nvpair-engine-manager/status_test.go index fb1fd0b96..8831bc2a8 100644 --- a/services/nvpair-engine-manager/status_test.go +++ b/services/nvpair-engine-manager/status_test.go @@ -244,8 +244,8 @@ func TestStartDoesNotWaitForPassiveProbe(t *testing.T) { ex := fixture.executor st, err := ex.state("fake") require.NoError(t, err) - st.plat.Detect = []string{fakeEngineBin} - st.plat.Runtime.Bin = fakeEngineBin + st.plat.Detect = []string{fakeEngineForTest(t)} + st.plat.Runtime.Bin = fakeEngineForTest(t) finished := make(chan statusResult, 1) go func() { status, err := ex.Status("fake") diff --git a/services/nvpair-engine-manager/testdata/fakeengine/main.go b/services/nvpair-engine-manager/testdata/fakeengine/main.go index dd241f3e3..379700e32 100644 --- a/services/nvpair-engine-manager/testdata/fakeengine/main.go +++ b/services/nvpair-engine-manager/testdata/fakeengine/main.go @@ -14,6 +14,7 @@ package main import ( "bufio" + "crypto/sha256" "encoding/json" "fmt" "log" @@ -22,6 +23,7 @@ import ( "os" "os/exec" "os/signal" + "path/filepath" "strconv" "strings" "sync" @@ -75,6 +77,30 @@ func bstr(m map[string]any, k string) string { } func main() { + // Registration is test-only and precedes every subcommand, including + // detached download children. It also follows copies of this executable. + if root := os.Getenv("PAIR_TEST_PROCESS_ROOT"); root != "" { + image, err := os.Executable() + if err != nil { + os.Exit(2) + } + image, err = filepath.EvalSymlinks(image) + if err != nil { + os.Exit(2) + } + key := fmt.Sprintf("%x", sha256.Sum256([]byte(image))) + owner, err := os.ReadFile(filepath.Join(root, "owners", key)) + if err != nil { + os.Exit(2) + } + record, _ := json.Marshal(struct { + PID int + Image string + }{os.Getpid(), image}) + if os.WriteFile(filepath.Join(string(owner), fmt.Sprintf("process-%d", os.Getpid())), record, 0600) != nil { + os.Exit(2) + } + } if path := os.Getenv("PAIR_TEST_ENV_FILE"); path != "" { values := map[string]string{} for _, key := range []string{"PAIR_TEST_CUSTOM", "PAIR_TEST_LITERAL", "PAIR_TEST_EMPTY", "OLLAMA_HOST"} { diff --git a/services/nvpair-engine-manager/testprocess_test.go b/services/nvpair-engine-manager/testprocess_test.go new file mode 100644 index 000000000..ba04f46e2 --- /dev/null +++ b/services/nvpair-engine-manager/testprocess_test.go @@ -0,0 +1,122 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "crypto/sha256" + "encoding/json" + "fmt" + "io/fs" + "os" + "path/filepath" + "strings" + "sync" + "testing" + "time" +) + +var testProcesses = struct { + sync.Mutex + dirs map[*testing.T]string +}{dirs: make(map[*testing.T]string)} + +// Every fake invocation has a per-test owner, including copied executables and +// detached grandchildren. The fake registers its PID before doing any work. +func fakeEngineForTest(t *testing.T) string { + t.Helper() + dir := fakeProcessDir(t) + bin := filepath.Join(dir, "fake-engine"+exeExt()) + if !fileExists(bin) { + data, err := os.ReadFile(fakeEngineBin) + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(bin, data, 0755); err != nil { + t.Fatal(err) + } + registerFakeExecutable(t, bin, dir) + } + return bin +} + +func fakeProcessDir(t *testing.T) string { + t.Helper() + testProcesses.Lock() + defer testProcesses.Unlock() + if dir := testProcesses.dirs[t]; dir != "" { + return dir + } + dir, err := os.MkdirTemp(testProcessRoot, "owner-") + if err != nil { + t.Fatal(err) + } + testProcesses.dirs[t] = dir + t.Cleanup(func() { + if remaining := sweepFakeProcesses(dir); len(remaining) != 0 { + t.Errorf("fake engines survived test cleanup: %+v", remaining) + } + testProcesses.Lock() + delete(testProcesses.dirs, t) + testProcesses.Unlock() + }) + return dir +} + +func registerFakeExecutable(t *testing.T, bin, owner string) { + t.Helper() + image, err := filepath.EvalSymlinks(bin) + if err != nil { + t.Fatal(err) + } + key := fmt.Sprintf("%x", sha256.Sum256([]byte(image))) + owners := filepath.Join(testProcessRoot, "owners") + if err := os.MkdirAll(owners, 0755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(owners, key), []byte(owner), 0600); err != nil { + t.Fatal(err) + } +} + +type fakeProcessRecord struct { + PID int + Image string +} + +func trackedFakeProcesses(root string) []fakeProcessRecord { + var alive []fakeProcessRecord + _ = filepath.WalkDir(root, func(path string, entry fs.DirEntry, err error) error { + if err != nil || entry.IsDir() || !strings.HasPrefix(entry.Name(), "process-") { + return nil + } + data, err := os.ReadFile(path) + if err != nil { + return nil + } + var record fakeProcessRecord + if json.Unmarshal(data, &record) == nil && record.PID > 0 && record.PID != os.Getpid() && pidAlive(record.PID) && + isOurEngineImage(testProcessImage(record.PID), record.Image) { + alive = append(alive, record) + } + return nil + }) + return alive +} + +func sweepFakeProcesses(root string) []fakeProcessRecord { + deadline := time.Now().Add(3 * time.Second) + for { + alive := trackedFakeProcesses(root) + if len(alive) == 0 { + return nil + } + for _, record := range alive { + killTestProcessGroup(record.PID) + } + if time.Now().After(deadline) { + return trackedFakeProcesses(root) + } + time.Sleep(10 * time.Millisecond) + } +} diff --git a/services/nvpair-engine-manager/testprocess_unix_test.go b/services/nvpair-engine-manager/testprocess_unix_test.go new file mode 100644 index 000000000..2a23c7edf --- /dev/null +++ b/services/nvpair-engine-manager/testprocess_unix_test.go @@ -0,0 +1,53 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +//go:build !windows + +package main + +import ( + "os/exec" + "strconv" + "strings" + "syscall" +) + +func testProcessImage(pid int) string { return procImage(pid) } + +func testRunProcesses(root string) []fakeProcessRecord { + out, err := exec.Command("ps", "-A", "-o", "pid=,args=").Output() + if err != nil { + return nil + } + var records []fakeProcessRecord + for _, line := range strings.Split(string(out), "\n") { + // Filter before the macOS lsof identity lookup: enumerating every system + // image would cost one subprocess per unrelated desktop process. + if !strings.Contains(line, root) && !strings.Contains(line, canonicalEnginePath(root)) { + continue + } + values := strings.Fields(line) + if len(values) == 0 { + continue + } + pid, err := strconv.Atoi(values[0]) + if err != nil { + continue + } + image := testProcessImage(pid) + if image != "" && isManagedInstallPath(canonicalEnginePath(image), canonicalEnginePath(root)) { + records = append(records, fakeProcessRecord{PID: pid, Image: image}) + } + } + return records +} + +func killTestProcessGroup(pid int) { + // A fixture without Setpgid may share the test runner's group. Never signal + // that group; kill only the owned process in that case. + if group, err := syscall.Getpgid(pid); err == nil && group != syscall.Getpgrp() { + _ = syscall.Kill(-group, syscall.SIGKILL) + } else { + _ = syscall.Kill(pid, syscall.SIGKILL) + } +} diff --git a/services/nvpair-engine-manager/testprocess_windows_test.go b/services/nvpair-engine-manager/testprocess_windows_test.go new file mode 100644 index 000000000..91be4f7d6 --- /dev/null +++ b/services/nvpair-engine-manager/testprocess_windows_test.go @@ -0,0 +1,33 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "os/exec" + "strconv" + "strings" +) + +func testProcessImage(pid int) string { return imagePathForPID(pid) } + +func testRunProcesses(root string) []fakeProcessRecord { + out, err := exec.Command("powershell.exe", "-NoProfile", "-NonInteractive", "-Command", "Get-Process | Select-Object -ExpandProperty Id").Output() + if err != nil { + return nil + } + var records []fakeProcessRecord + for _, value := range strings.Fields(string(out)) { + pid, err := strconv.Atoi(value) + if err != nil { + continue + } + image := testProcessImage(pid) + if image != "" && isManagedInstallPath(canonicalEnginePath(image), canonicalEnginePath(root)) { + records = append(records, fakeProcessRecord{PID: pid, Image: image}) + } + } + return records +} + +func killTestProcessGroup(pid int) { _ = signalPID(pid, true) } diff --git a/services/nvpair-engine-manager/uninstall_coverage_test.go b/services/nvpair-engine-manager/uninstall_coverage_test.go index 712e0ce39..4ed7e66f2 100644 --- a/services/nvpair-engine-manager/uninstall_coverage_test.go +++ b/services/nvpair-engine-manager/uninstall_coverage_test.go @@ -54,7 +54,7 @@ func TestCommandUninstallCoverage(t *testing.T) { } p.Uninstall.Run = rmCmd(target) stopMarker := filepath.Join(home, "stop-ran") - p.Runtime.Stop = &StopSpec{Cmd: []string{fakeEngineBin, "touch", stopMarker}} + p.Runtime.Stop = &StopSpec{Cmd: []string{fakeEngineForTest(t), "touch", stopMarker}} m.Platforms[key] = p ex := reconciledTestExecutor(NewExecutor(reg, NewReporter(nil), nil, base)) ex.detectTimeout = 0 // never wait for a destructive-command regression diff --git a/services/nvpair-engine-manager/worker_health_test.go b/services/nvpair-engine-manager/worker_health_test.go index 780ff1bc8..f1181d48d 100644 --- a/services/nvpair-engine-manager/worker_health_test.go +++ b/services/nvpair-engine-manager/worker_health_test.go @@ -20,7 +20,7 @@ func TestWorkerHealth(t *testing.T) { for _, rawID := range []string{"7", `"health-7"`} { t.Run(rawID, func(t *testing.T) { var out bytes.Buffer - ex := newTestExecutor(t, testEngineManifest(fakeEngineBin)) + ex := newTestExecutor(t, testEngineManifest(fakeEngineForTest(t))) st, err := ex.state("fake") if err != nil { t.Fatal(err) From 9a6dbbb861314480da3b040a17e942a6e190dbe2 Mon Sep 17 00:00:00 2001 From: Jared Ping <13906915+JaredP94@users.noreply.github.com> Date: Fri, 6 Nov 2026 12:37:12 +0200 Subject: [PATCH 5/7] fix(engine-manager): add unsupported target build fallbacks FreeBSD compilation had no fileIdentity or watchDownloadExit implementation. Fail closed for install identity and use ordinary reaping on unsupported targets, without claiming engine or platform support. Signed-off-by: Jared Ping <13906915+JaredP94@users.noreply.github.com> --- services/nvpair-engine-manager/README.md | 7 +++++++ .../nvpair-engine-manager/download_wait_other.go | 14 ++++++++++++++ services/nvpair-engine-manager/fileid_other.go | 10 ++++++++++ 3 files changed, 31 insertions(+) create mode 100644 services/nvpair-engine-manager/download_wait_other.go create mode 100644 services/nvpair-engine-manager/fileid_other.go diff --git a/services/nvpair-engine-manager/README.md b/services/nvpair-engine-manager/README.md index ad867e64b..e0d8ed5cf 100644 --- a/services/nvpair-engine-manager/README.md +++ b/services/nvpair-engine-manager/README.md @@ -144,6 +144,13 @@ The bundled uninstalls remove only what an install put down: Ollama's app or install directory, and LM Studio's `~/.lmstudio/bin` and `llmster` runtime. LM Studio's conversations, credentials and settings are left where they are. +### Unsupported-target build fallbacks + +FreeBSD can compile the engine manager, but is not a supported platform and has +no bundled engine manifests. Its fallbacks refuse app install-identity claims +and use ordinary process waiting rather than detached-downloader exit watching. +This does not extend the supported platform list. + ### Fake-engine test cleanup Each fake-engine executable (including test copies) has a per-test owner under diff --git a/services/nvpair-engine-manager/download_wait_other.go b/services/nvpair-engine-manager/download_wait_other.go new file mode 100644 index 000000000..21b3af7c1 --- /dev/null +++ b/services/nvpair-engine-manager/download_wait_other.go @@ -0,0 +1,14 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +//go:build !darwin && !linux && !windows + +package main + +// Unsupported targets have no non-reaping exit watcher. Use ordinary Wait; +// this is a build fallback, not supported detached-downloader supervision. +func watchDownloadExit(w *downloadWait) error { + w.reap() + go func() { <-w.finished; close(w.exited) }() + return nil +} diff --git a/services/nvpair-engine-manager/fileid_other.go b/services/nvpair-engine-manager/fileid_other.go new file mode 100644 index 000000000..763152e3b --- /dev/null +++ b/services/nvpair-engine-manager/fileid_other.go @@ -0,0 +1,10 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +//go:build !darwin && !linux && !windows + +package main + +// Unsupported targets cannot prove a generation-stable install identity. +// Fail closed: a claim naming a created app never authorizes its removal. +func fileIdentity(string) string { return "" } From 7fea52940cdbb814341c1c12958cacdb92a65c06 Mon Sep 17 00:00:00 2001 From: Jared Ping <13906915+JaredP94@users.noreply.github.com> Date: Fri, 6 Nov 2026 14:33:15 +0200 Subject: [PATCH 6/7] fix(engine-manager): snapshot health probes under the state lock Settings rollback replaces the entire runtime while a canceled health goroutine may still be starting. Read and copy its probe under the same state mutex used by rollback, then release the mutex before timers or network probes. Add a concurrent runtime-restoration regression that exposes the race on both main and the original PR head. The existing rollback test passed 50 race-enabled repetitions on both revisions; after the fix it still passes 50 repetitions, as does the new regression. Two uncached full race suites and darwin, linux, windows, and freebsd vet checks pass. Keep the PR's existing engine-manager version bump. Signed-off-by: Jared Ping <13906915+JaredP94@users.noreply.github.com> --- services/nvpair-engine-manager/lifecycle.go | 9 +++-- .../lifecycle_health_test.go | 33 +++++++++++++++++++ 2 files changed, 40 insertions(+), 2 deletions(-) create mode 100644 services/nvpair-engine-manager/lifecycle_health_test.go diff --git a/services/nvpair-engine-manager/lifecycle.go b/services/nvpair-engine-manager/lifecycle.go index ba7cc8489..68b98aa0d 100644 --- a/services/nvpair-engine-manager/lifecycle.go +++ b/services/nvpair-engine-manager/lifecycle.go @@ -914,7 +914,12 @@ func (e *Executor) stopEngines(names []string) []string { } func (e *Executor) runHealth(ctx context.Context, st *engineState, engine string, port int, gen int64) { - h := st.plat.Runtime.Health + // Settings rollback can replace Runtime after this loop was canceled but + // before its goroutine starts. Keep a private probe snapshot, and release + // mu before waiting or performing any network I/O. + st.mu.Lock() + h := *st.plat.Runtime.Health + st.mu.Unlock() interval := 5 * time.Second if h.IntervalS > 0 { interval = time.Duration(h.IntervalS) * time.Second @@ -926,7 +931,7 @@ func (e *Executor) runHealth(ctx context.Context, st *engineState, engine string case <-ctx.Done(): return case <-t.C: - ok := e.probe(ctx, h, port) + ok := e.probe(ctx, &h, port) st.mu.Lock() // Bail if stopped or if a newer start superseded this loop — // a stale loop must never write health for a later run. diff --git a/services/nvpair-engine-manager/lifecycle_health_test.go b/services/nvpair-engine-manager/lifecycle_health_test.go new file mode 100644 index 000000000..1b79f0045 --- /dev/null +++ b/services/nvpair-engine-manager/lifecycle_health_test.go @@ -0,0 +1,33 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "context" + "sync" + "testing" +) + +func TestHealthStartupConcurrentRuntimeRestore(t *testing.T) { + e := &Executor{} + previous := Runtime{Health: &Probe{IntervalS: 1}} + st := &engineState{plat: &Platform{Runtime: previous}} + // Stop cancels the old loop but does not join its goroutine. Settings + // rollback can therefore restore the whole Runtime before that loop + // has even read its health configuration. + ctx, cancel := context.WithCancel(context.Background()) + cancel() + var loops sync.WaitGroup + for range 1000 { + loops.Add(1) + go func() { + defer loops.Done() + e.runHealth(ctx, st, "fake", 0, 0) + }() + st.mu.Lock() + st.plat.Runtime = previous + st.mu.Unlock() + } + loops.Wait() +} From ee9f6ce01cee5b2925fd2270147da5c0e99839e2 Mon Sep 17 00:00:00 2001 From: Jared Ping <13906915+JaredP94@users.noreply.github.com> Date: Fri, 6 Nov 2026 15:14:17 +0200 Subject: [PATCH 7/7] fix(engine-manager): snapshot platform configuration for unlocked readers Settings rollback replaces Runtime under the state mutex while an already admitted passive status probe can still be detecting executables. Revision fencing prevents stale publication but does not synchronize those reads. Add engineState.platformSnapshot and use it for passive status/Detect, CLI and remove-path actions, profile residency observations, health startup and live-platform capabilities. Move capability filesystem probes outside mu. The copy is shallow: Detect, DetectRequires, Install, Uninstall, and all Runtime slice/map/pointer referents (Ready, Health, Stop, EditableLaunch, Args, Env, Start, LaunchArgs and LaunchEnv) are immutable after publication. Settings replace argument/environment pointers or Runtime as a whole; SetPort replaces only Port. installDir/modelsDir are fixed at construction. Test fixtures that customize referents do so before concurrent consumers. Audit of direct platform readers retained with their existing locks: - st.mu: platformSnapshot; refreshPassiveStatus's Ready and mode captures; reconcilePresence's Ready capture; stopEngines's shutdown mode check; launchStateLocked's ServerPort capture; SetPort's platform nil checks and Port restoration/update; ConfigureLaunch's apply/rollback writes. - st.opMu: Install's mode and Install reads; Uninstall's Uninstall/mode reads and installCreates(st.plat); doStart's port/Runtime reads and controlCLIPath(st.plat); doStop's Runtime capture; runCommandStop's controlCLIPath(st.plat); reconcileFailedCommandStop's Runtime capture; reconcilePresence's mode check; runLMSLoad's controlCLIPath(st.plat) (called by locked lmsModelAction or doStart -> restoreLMSModels); launchForState's Runtime; launchStateLocked's editable-launch and adopted-policy checks; previewLaunchLocked's port/Runtime; and ConfigureLaunch's previousRuntime, stopAttempt Runtime and persistence comparisons. All settings apply/rollback paths hold the same opMu. - state initializes from a local Platform copy before publication under Executor.mu. Registry PlatformFor/resolveEngine/capabilitiesFor consume the immutable registry, not engineState's replaceable Runtime. Readiness uses the Runtime captured by doStart under opMu; its cancellation watcher reads no platform. Process-output callbacks retain that private Runtime copy; exit watchers and process-stop goroutines read no platform. Command-stop cleanup remains synchronous under opMu. All three production health launch sites check Health first, and settings updates preserve that immutable pointer, including rollback. runHealth additionally snapshots and returns safely for nil Health instead of relying on the caller invariant. TestPassiveStatusConcurrentRuntimeRestore reproduces the status and Detect races on 7fea529 under -race. TestHealthStartupWithoutProbe reproduces the previous nil dereference. Keep TestSettingsApplyRestoresPreviousState and TestHealthStartupConcurrentRuntimeRestore unchanged. Validate the four regressions with -race -count=30, run the full race suite twice without the test cache, and vet darwin/linux/windows/freebsd. No additional version bump: the PR already contains the engine-manager bump. Signed-off-by: Jared Ping <13906915+JaredP94@users.noreply.github.com> --- services/nvpair-engine-manager/README.md | 4 + services/nvpair-engine-manager/actions.go | 8 +- .../nvpair-engine-manager/capabilities.go | 6 +- services/nvpair-engine-manager/executor.go | 14 ++++ services/nvpair-engine-manager/lifecycle.go | 8 +- .../loadprofiles_residency.go | 3 +- services/nvpair-engine-manager/status.go | 36 ++++++--- .../nvpair-engine-manager/status_race_test.go | 78 +++++++++++++++++++ 8 files changed, 137 insertions(+), 20 deletions(-) create mode 100644 services/nvpair-engine-manager/status_race_test.go diff --git a/services/nvpair-engine-manager/README.md b/services/nvpair-engine-manager/README.md index e0d8ed5cf..39b6ccb5c 100644 --- a/services/nvpair-engine-manager/README.md +++ b/services/nvpair-engine-manager/README.md @@ -407,6 +407,10 @@ lifecycle lock. A normal status read during an operation returns its current snapshot; explicit port checks and model sweeps wait within their caller's budget. Observations are discarded if a lifecycle operation or a newer state change intervened, so a delayed readiness reply cannot undo stop or set-port. +Platform readers outside the engine operation lock copy the configuration under +the state mutex before detection, actions, profile observations or health work; +settings rollback cannot race those reads. Filesystem capability checks also run +outside the state mutex. The five-second model sweep budget includes status probes and operation waits. `GET /v1/models` returns HTTP 503 if the sweep cannot complete in time, rather diff --git a/services/nvpair-engine-manager/actions.go b/services/nvpair-engine-manager/actions.go index 365a4588b..2927e85dc 100644 --- a/services/nvpair-engine-manager/actions.go +++ b/services/nvpair-engine-manager/actions.go @@ -196,7 +196,8 @@ func (e *Executor) runRemovePathAction(ctx context.Context, st *engineState, act root = expandPath(root) if act.ModelResolution == modelResolutionLMSDiskPath { - cli := controlCLIPath(st.plat, st.installDir, runtime.GOOS) + plat, installDir := st.platformSnapshot() + cli := controlCLIPath(&plat, installDir, runtime.GOOS) targets, err := e.lmsDiskPathsToDelete(ctx, cli, root, vars["model"]) if err != nil { return nil, err @@ -252,8 +253,9 @@ func (e *Executor) runCmdAction(ctx context.Context, st *engineState, act Action } } vars["port"] = strconv.Itoa(port) - vars["install_dir"] = st.installDir - if cli := controlCLIPath(st.plat, st.installDir, runtime.GOOS); cli != "" { + plat, installDir := st.platformSnapshot() + vars["install_dir"] = installDir + if cli := controlCLIPath(&plat, installDir, runtime.GOOS); cli != "" { vars["cli"] = cli } diff --git a/services/nvpair-engine-manager/capabilities.go b/services/nvpair-engine-manager/capabilities.go index b502d3def..4c4bcee17 100644 --- a/services/nvpair-engine-manager/capabilities.go +++ b/services/nvpair-engine-manager/capabilities.go @@ -41,6 +41,11 @@ func capabilitiesFor(m *Manifest, platform string, adopted bool, installDir stri if !ok { return Capabilities{} } + goos := strings.SplitN(platform, "/", 2)[0] + return capabilitiesForPlatform(m, p, goos, adopted, installDir) +} + +func capabilitiesForPlatform(m *Manifest, p Platform, goos string, adopted bool, installDir string) Capabilities { _, list := m.Actions["list_models"] _, pull := m.Actions["pull_model"] deleteAction, deleteModel := m.Actions["delete_model"] @@ -48,7 +53,6 @@ func capabilitiesFor(m *Manifest, platform string, adopted bool, installDir stri if uninstall && p.Runtime.modeOrDefault() == "command" { // Uninstall declines a command-mode engine PAIR has no record of // installing, so offering it would only lead to that refusal. - goos := strings.SplitN(platform, "/", 2)[0] creates := "" if p.Install != nil { creates = p.Install.Creates diff --git a/services/nvpair-engine-manager/executor.go b/services/nvpair-engine-manager/executor.go index f30464359..11532024b 100644 --- a/services/nvpair-engine-manager/executor.go +++ b/services/nvpair-engine-manager/executor.go @@ -83,6 +83,20 @@ type engineState struct { restoreModels map[string]bool } +// platformSnapshot copies the mutable platform under mu; callers must release +// mu before invoking it, and perform probes/I/O only after it returns. +// A shallow copy is safe: Detect/DetectRequires, Install/Uninstall, and Runtime's +// slices, maps and pointers (including Ready, Health, Stop and EditableLaunch) +// are immutable after publication. Settings replace LaunchArgs/LaunchEnv or the +// whole Runtime, never their referents; SetPort only replaces Runtime.Port. +// installDir (like modelsDir) is fixed when state is created; include it so +// consumers resolve paths from the same configuration snapshot. +func (st *engineState) platformSnapshot() (Platform, string) { + st.mu.Lock() + defer st.mu.Unlock() + return *st.plat, st.installDir +} + // lockOperation marks the entire serialized operation, including external I/O. func (st *engineState) lockOperation() { st.opMu.Lock() diff --git a/services/nvpair-engine-manager/lifecycle.go b/services/nvpair-engine-manager/lifecycle.go index 68b98aa0d..d56c9ea9d 100644 --- a/services/nvpair-engine-manager/lifecycle.go +++ b/services/nvpair-engine-manager/lifecycle.go @@ -917,9 +917,11 @@ func (e *Executor) runHealth(ctx context.Context, st *engineState, engine string // Settings rollback can replace Runtime after this loop was canceled but // before its goroutine starts. Keep a private probe snapshot, and release // mu before waiting or performing any network I/O. - st.mu.Lock() - h := *st.plat.Runtime.Health - st.mu.Unlock() + plat, _ := st.platformSnapshot() + if plat.Runtime.Health == nil { + return + } + h := *plat.Runtime.Health interval := 5 * time.Second if h.IntervalS > 0 { interval = time.Duration(h.IntervalS) * time.Second diff --git a/services/nvpair-engine-manager/loadprofiles_residency.go b/services/nvpair-engine-manager/loadprofiles_residency.go index 2aff6325e..1efbadd18 100644 --- a/services/nvpair-engine-manager/loadprofiles_residency.go +++ b/services/nvpair-engine-manager/loadprofiles_residency.go @@ -45,7 +45,8 @@ func (e *Executor) observeLoadProfile(ctx context.Context, snap *settings.LoadPr return } act := st.manifest.Actions["load_model"] - argv, err := resolveArgs(act.Cmd, map[string]string{"model": snap.Model, "port": strconv.Itoa(port), "install_dir": st.installDir, "cli": controlCLIPath(st.plat, st.installDir, runtime.GOOS)}) + plat, installDir := st.platformSnapshot() + argv, err := resolveArgs(act.Cmd, map[string]string{"model": snap.Model, "port": strconv.Itoa(port), "install_dir": installDir, "cli": controlCLIPath(&plat, installDir, runtime.GOOS)}) if err != nil || len(argv) == 0 { return } diff --git a/services/nvpair-engine-manager/status.go b/services/nvpair-engine-manager/status.go index a4b59da56..4d252da24 100644 --- a/services/nvpair-engine-manager/status.go +++ b/services/nvpair-engine-manager/status.go @@ -62,7 +62,8 @@ func (e *Executor) Detect(engine string) (bool, error) { // detectPath observes the filesystem without publishing a partial state. A // passive status check applies this result together with its port observation. func detectPath(st *engineState) (bool, string) { - binPath := detectedExecutable(st.plat, st.installDir, runtime.GOOS) + plat, installDir := st.platformSnapshot() + binPath := detectedExecutable(&plat, installDir, runtime.GOOS) return binPath != "", binPath } @@ -113,8 +114,9 @@ func (e *Executor) statusWithContext(ctx context.Context, engine string, probePo } } result := statusLocked(engine, st) + adopted := st.adopted st.mu.Unlock() - return result, nil + return e.withCapabilities(result, st, adopted), nil } key := statusProbeKey{revision: st.revision, port: port} if st.statusRuns == nil { @@ -162,8 +164,9 @@ func (e *Executor) statusWithContext(ctx context.Context, engine string, probePo continue } result := statusLocked(engine, st) + adopted := st.adopted st.mu.Unlock() - return result, nil + return e.withCapabilities(result, st, adopted), nil } return e.snapshot(engine, st), nil } @@ -195,19 +198,28 @@ func (e *Executor) Errors() []serviceError { func (e *Executor) snapshot(engine string, st *engineState) EngineStatus { st.mu.Lock() - defer st.mu.Unlock() - return statusLocked(engine, st) + result := statusLocked(engine, st) + adopted := st.adopted + st.mu.Unlock() + return e.withCapabilities(result, st, adopted) +} + +// Capability derivation may probe installation markers and executable paths. +// Keep that I/O outside mu and use the live platform, not the registry original. +func (e *Executor) withCapabilities(result EngineStatus, st *engineState, adopted bool) EngineStatus { + plat, installDir := st.platformSnapshot() + result.Capabilities = capabilitiesForPlatform(st.manifest, plat, runtime.GOOS, adopted, installDir) + return result } func statusLocked(engine string, st *engineState) EngineStatus { return EngineStatus{ - Engine: engine, - DisplayName: st.manifest.DisplayName, - Installed: st.installed, - Running: st.running, - Healthy: st.healthy, - Port: st.port, - Capabilities: capabilitiesFor(st.manifest, runtime.GOOS+"/"+runtime.GOARCH, st.adopted, st.installDir), + Engine: engine, + DisplayName: st.manifest.DisplayName, + Installed: st.installed, + Running: st.running, + Healthy: st.healthy, + Port: st.port, } } diff --git a/services/nvpair-engine-manager/status_race_test.go b/services/nvpair-engine-manager/status_race_test.go new file mode 100644 index 000000000..7a1dc3acb --- /dev/null +++ b/services/nvpair-engine-manager/status_race_test.go @@ -0,0 +1,78 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "context" + "runtime" + "sync" + "testing" +) + +// A passive probe can outlive its revision and overlap settings rollback. +// Revision fencing must not leave detectPath reading the mutable Runtime. +func TestPassiveStatusConcurrentRuntimeRestore(t *testing.T) { + for _, detect := range []bool{false, true} { + name := "status" + if detect { + name = "detect" + } + t.Run(name, func(t *testing.T) { + e := newTestExecutor(t, testEngineManifest(fakeEngineForTest(t))) + st, err := e.state("fake") + if err != nil { + t.Fatal(err) + } + // Command mode exercises both Mode and CLI in detectedExecutable. + st.plat.Runtime.Mode = "command" + st.plat.Runtime.CLI = fakeEngineForTest(t) + previous := st.plat.Runtime + // Use rollback's locks and whole-value assignment. Leave opDone + // clear to exercise an already-admitted passive probe on every read. + var writer sync.WaitGroup + start, stop := make(chan struct{}), make(chan struct{}) + writer.Add(1) + go func() { + defer writer.Done() + st.opMu.Lock() + defer st.opMu.Unlock() + close(start) + for { + select { + case <-stop: + return + default: + } + st.mu.Lock() + st.plat.Runtime = previous + st.mu.Unlock() + runtime.Gosched() + } + }() + <-start + defer writer.Wait() + defer close(stop) + for range 100 { + var installed bool + if detect { + installed, err = e.Detect("fake") + } else { + var status EngineStatus + status, err = e.statusWithContext(context.Background(), "fake", 0, false) + installed = status.Installed + } + if err != nil || !installed { + t.Fatalf("passive detection lost installed engine: installed=%v err=%v", installed, err) + } + } + }) + } +} + +func TestHealthStartupWithoutProbe(t *testing.T) { + st := &engineState{plat: &Platform{}} + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + (&Executor{}).runHealth(ctx, st, "fake", 0, 0) +}