diff --git a/docs/known-issues.mdx b/docs/known-issues.mdx index 11faae7b8..ed9a00caf 100644 --- a/docs/known-issues.mdx +++ b/docs/known-issues.mdx @@ -120,22 +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 -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 97ff1b3ee..39b6ccb5c 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 @@ -137,6 +144,39 @@ 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 +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 +[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, @@ -367,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 @@ -554,6 +598,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/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/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 b81237de4..f2ff751e5 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 := "" @@ -197,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", @@ -221,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.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/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/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/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.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/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/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 "" } 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.go b/services/nvpair-engine-manager/lifecycle.go index ea047bdcb..d56c9ea9d 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 @@ -891,7 +914,14 @@ 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. + 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 @@ -903,7 +933,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() +} 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/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/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/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/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/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/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) + } + }) + } +} 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/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/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/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/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} 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 new file mode 100644 index 000000000..c4a06325f --- /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(fakeEngineForTest(t), "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(fakeEngineForTest(t)) + p := m.Platforms[hostKey()] + p.Runtime.Mode = "command" + p.Runtime.Port = port + p.Runtime.Stop = &StopSpec{Cmd: []string{fakeEngineForTest(t), "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/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.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) +} 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 499c5c4c1..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"} { @@ -180,6 +206,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) 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)