fix(worker): reap deleted backends and stop models that live on a worker

[localai-bot]

Three related backend-lifecycle defects from one production incident on a Jetson/Thor worker: a deleted backend's gRPC process survived ~40 minutes with its directory removed from disk, a later model load was routed to that orphan and failed with a certifi path pointing into the deleted directory, and the admin could not stop the model because the frontend reported it as not loaded.

1. backend.delete orphaned the process it claimed to delete

s.processes is keyed modelID#replicaIndex, so the backend name appears in no key and was recorded nowhere on the process. A delete keyed on a backend name resolved to zero keys via resolveProcessKeys (whose prefix path only matches a bare modelID), the stop silently no-op'd, and the files were removed out from under a live process — while the reply reported success and the endpoint returned 200.

Aliasing made a naive name match insufficient on its own: the model config referenced the alias (longcat-video) while the delete carried the concrete directory name (cuda13-nvidia-l4t-arm64-longcat-video).

• records backendName on backendProcess
• adds resolveProcessKeysForBackend, resolving alias↔concrete via ListSystemBackends before DeleteBackendFromSystem erases the metadata carrying the alias; resolution failure degrades to name-only matching so a delete never fails on it
• gates the installBackend fast path on processMatchesBackend, so a reinstalled variant cannot inherit a deleted backend's port (processes with no recorded name, i.e. pre-upgrade, are still accepted so a rollout doesn't restart every running backend)
• routes backend.stop through resolveStopTargets, which accepts a backend name, a model name, or an exact modelID#replica key — all three are published in practice: the admin UI sends a backend name, UnloadRemoteModel sends a model name, and fix(distributed): configurable remote model-load timeout, and reap the load when it times out #10948's reap sends an exact replica key. backend.delete stays strict, since its identifier is unambiguously a backend name.

2. /backend/shutdown reported a running model as missing

POST /backend/shutdown returned 500 model not found for a model demonstrably loaded on worker nvidia-thor with a live backend process.

ModelLoader.deleteProcess short-circuits on a miss in the replica's in-memory store. In distributed mode the authoritative record is the shared node registry, so any frontend replica that never served the model itself has no local entry. The remote-unload path documented at pkg/model/loader.go:28-34 sits below that short-circuit — unreachable in exactly the case it exists for. #10865 reworked this function but kept the short-circuit intact (pkg/model/process.go:59-63).

• consults the remote unloader on a local-store miss, via a shared unloadRemote helper so both the miss path and the existing no-process path honour RemoteModelContextUnloader and preserve fix(model): make backend shutdown model-scoped #10865's force propagation
• UnloadRemoteModelContext reports ErrRemoteModelNotLoaded when no node has the model — it previously returned nil, indistinguishable from a successful stop
• the endpoint now returns 404 naming both scopes ("not loaded on this instance or on any worker node") instead of a blanket 500, plus 400 on an empty model and a JSON success body. Without this the loader-side fix is invisible to the operator.

3. Coverage for the bounded Free()

#10865 added workerBackendFreeTimeout (supervisor.go:30,243, lifecycle.go:261) with no test coverage, and that bound is load-bearing for the reap merged in #10948: stopBackendExact calls Free before proc.Stop(), and 37 Python backends default to PYTHON_GRPC_MAX_WORKERS=1, so a backend wedged in LoadModel has no thread to service it.

This branch originally carried its own bounding change; it is dropped as superseded — master's version is strictly better, as it releases the supervisor mutex across the Free and keeps the port reserved until termination. Only the regression test survives.

The spec stands up a real pb.RegisterBackendServer whose Free handler blocks forever. A stub socket that accepted TCP without speaking gRPC was tried first and passed against the very bug it targets — gRPC's own ~20s connect timeout was bounding the call because the handshake never completed.

Notes for reviewers

• Rebased onto current master. The original three commits were collapsed into one because their boundaries reflected the pre-fix(model): make backend shutdown model-scoped #10865 base and had become misleading — one of them would have carried a message describing a production change whose diff no longer existed.
• The free_timeout_test.go fixture process is deliberately never started. go-processmanager v0.1.1 has a data race — readPID() writes Process.pid unsynchronized, reached from Stop(), IsAlive(), and its own monitor goroutine. Starting a real process would turn the fail-closed -race conformance gate red on an upstream defect. Filed as data race: readPID() writes Process.pid unsynchronized, reachable from Stop(), IsAlive() and the internal monitor goroutine go-processmanager#5. The tradeoff is that the spec proves the stop is reached and the slot released — which is exactly what an unbounded Free prevents — but not that SIGTERM/SIGKILL landed, which is go-processmanager's contract rather than the supervisor's.

Known risk

Reaping on delete now recycles the backend's port via freePorts, which the next startBackend may immediately reuse. A controller holding the old address can then pass probeHealth against a different backend on that port, since the probe verifies liveness rather than identity. Narrow, and strictly better than routing into a deleted directory, but the failure is silent — tracked as #10952.

Verification

make lint (new-from origin/master)          0 issues.
coverage gate                                48.5% -> 53.4%
scripts/model-lifecycle-conformance.sh:
  [1/3] pkg/model 2.952s | pkg/grpc 2.191s | core/services/worker 6.602s   (-race)
  [2/3] core/services/nodes 1.241s
  [3/3] FizzBee: Valid Nodes 1458, IsLive: true, PASSED

---

🤖 Generated with Claude Code

[localai-bot]

Second commit 2b7a04936 partially reverts a contract change from the first, after CI caught it. Recording why here, since the diff alone does not explain it.

What CI caught. tests/e2e/distributed/node_lifecycle_test.go:145 — "should be no-op for models not on any node" — failed with model not loaded on any node. The first commit had UnloadRemoteModel return a new ErrRemoteModelNotLoaded where it previously returned nil, so /backend/shutdown could tell "not loaded anywhere" from "stopped successfully" and answer 404 instead of a misleading 500.

Why the spec was right and the change was wrong. That spec deliberately pins idempotent no-op semantics, and a caller audit found the e2e failure was the least serious consequence:

• pkg/model/watchdog.go:902 — the LRU reclaimer. It untracks a model only when shutdown returns nil or ErrModelNotFound. That is an allow-list, so a new error type skips the untrack entirely, the reclaimer re-selects the same entry forever, and memory is never reclaimed. A permanent wedge in exactly the distributed deployments this PR targets — and no test would have caught it.
• pkg/model/process.go — the no-local-process branch returns the unload result directly, so a stale local entry for a model no longer on any node turned successful cleanup into failure.
• galleryop/managers_local.go DeleteModel, and modeladmin/{state,config,remote_sync}.go — best-effort stops that legitimately run against already-unloaded models would have started erroring or warning.

Only one production caller of UnloadRemoteModel exists, and only its store-miss path wanted the distinction. Narrowing a shared adapter's contract to serve one call site was the wrong shape.

The fix. ErrRemoteModelNotLoaded is removed and the adapter returns nil again. A new optional RemoteModelPresenceChecker.HasRemoteModel answers presence directly, consulted before unloading — an idempotent unload cannot report afterwards whether anything was stopped. A failed registry lookup surfaces as an error rather than as absence, since answering a confident 404 off a failed lookup is the same class of bug as the original 500.

The e2e spec is unchanged and passes as originally written. The only edit to that file is a comment cross-referencing the unit-level spec, because the two disagreeing about the same contract is what let this reach CI. Both now name each other and state the watchdog dependency, so the next person changing it sees the cost.

Also added compile-time assertions for the three optional interfaces in the nodes package — they are consumed by runtime type assertion, where a signature drift silently downgrades behavior (losing force propagation, or the presence check) instead of failing the build. That is the same failure mode as the one that bit here.

Verified locally before push: ginkgo --label-filter='Distributed && !VLLMMultinode' -r ./tests/e2e/distributed → 233 passed, 0 failed, 1 skipped (the suite CI failed 232/1 on). Coverage 48.5% → 53.4%. make lint 0 issues.

[localai-bot]

Third commit 4843d57c8 — closes the "Known risk" this PR documents, so the window never exists on master. Tracked as #10952.

Folding it in here rather than as a follow-up PR: this PR is what makes port recycling reachable (before it, backend.delete never actually stopped a process, so a delete never freed a port), and shipping the mitigation separately would leave a silent-misroute window open between the two merges.

The framing that makes the bug obvious: UnloadModel (core/services/nodes/router.go:1550-1560) calls RemoveAllNodeModelReplicas immediately after StopBackend. DeleteBackend and UpgradeBackend never remove rows at all. Delete and upgrade are the two paths that recycle a port without telling anyone; unload is the one that gets it right.

upgradeBackend (core/services/worker/install.go:186-200) force-stops every process for the backend and has the identical gap — that was not in the original issue and was found during this work.

Why not identity verification in probeHealth (the approach #10952 originally suggested as more complete), rejected on three independent grounds, recorded so it is not rediscovered:

• no identity exists to assert on — Health returns Reply (backend.proto:337-346), Status returns StatusResponse (:682-691), HealthMessage is empty (:262), so there is nowhere to send an expected identity either. It needs a proto change plus implementations in 36 Python and 4 C++ servicers.
• fail-open during rolling upgrades — the controller must treat an empty identity as "allow", so the bug persists for every backend not yet rebuilt.
• decisive: it would never execute in the failure window. probeHealth short-circuits on probeCache (probe_cache.go:75-92, 30s TTL, keyed nodeID|addr), and the address was probed successfully while the old backend was alive — so for up to 30s after the port is rebound, DoOrCached returns true with no round-trip. Always executing it defeats a cache that exists to fix a real queue-stalling bug.

The fix. StoppedProcessKeys []string + ReportsStoppedProcesses bool on BackendDeleteReply / BackendUpgradeReply; the controller drops rows in RemoteUnloaderAdapter using the registry ModelLocator and RemoveNodeModel it already held. No new interface, no migration, no proto change, and all three call sites funnel through the adapter.

Two contract details worth reviewer attention:

• A key is reported only after its process is confirmed dead, so the list is trustworthy even on failure replies and rows drop regardless of Success. This matters: handleBackendDelete has two failure paths after the stop loop (file removal, re-registration) where processes are already dead and ports already recycled — gating removal on Success would have stranded exactly those rows. Conversely the abort-on-stop-failure path reports nothing, so a surviving process keeps its row.
• Upgrade's speculative bare-backend key is filtered out. upgradeBackend appends buildProcessKey("", req.Backend, replica), which usually matches nothing; reporting it would ask the controller to delete a row keyed on the backend name, which could collide with a model whose ID equals it.

Old workers. ReportsStoppedProcesses is what makes an empty list interpretable — false means "predates this field", and the adapter leaves rows alone and falls back to probe-based recovery. Specs cover old-worker delete, old-worker upgrade, new-worker-with-nothing-stopped, and raw-JSON decode of a legacy reply (fed as raw JSON precisely so they can express a shape the current struct cannot produce).

Port quarantine, 15s, swept lazily on allocation, as an interlock for the NATS round-trip window between the worker freeing the port and the controller dropping the row. The comment states explicitly that it is not derived from HealthCheckInterval and why that coupling would be unsound (the interval is operator-configurable and DisablePerModelHealthCheck can switch the reaper off entirely), and that raising it is not a substitute for eager row removal.

model.ParseBackendProcessKey sits beside BackendProcessKey and shares its separator. It splits at the last separator, not the first — model IDs may contain #, and a first-split would address a nonexistent row while leaving the real stale one in place.

Three pre-existing assertions in replica_test.go that asserted immediate freePorts reuse were rewritten to assert the quarantine holds the port — an intentional behavior change, not an accommodation.

Verification: make lint 0 issues; coverage gate OK (53.4% vs 48.5% baseline); Worker Suite 85 passed / 0 failed (81 pre-existing + 4 new quarantine specs); no real process started under -race, so go-processmanager#5 stays out of the fail-closed conformance gate.

Not done, deliberately: the coverage gate suggests make test-coverage-baseline (53.4% vs a committed 48.5%). The baseline file is untouched — moving the ratchet is a separate deliberate act, and +4.9pp is larger than these changes plausibly account for, so the baseline may be stale from earlier truncated runs on this box.

[localai-bot]

Fourth commit 957c3d5f2 — bumps go-processmanager and uses the bump to restore an assertion this PR had to give up.

Why the bump. This PR's free_timeout_test.go originally left its fixture process deliberately unstarted, because starting a real process under -race tripped a data race in go-processmanager v0.1.1 (readPID() writing Process.PID unsynchronized while the library's own monitor goroutine read it) and would have turned the fail-closed scripts/model-lifecycle-conformance.sh gate red on an upstream bug. The recorded cost: the spec could prove the stop was reached and the slot released, but not that SIGTERM/SIGKILL actually landed.

That race is now fixed upstream (mudler/go-processmanager#6). Pinned to v0.1.2-0.20260719202549-a94e2b7e5f4f — a pseudo-version, as upstream has not cut a tag past v0.1.1.

Migration. The upstream fix deprecated the exported PID field (a direct read still races with the monitor clearing it on exit) and added CurrentPID(). pkg/model/process.go:224 (GetGRPCPID) was the only direct .PID read on a go-processmanager Process in the tree — confirmed by grepping all *.go including tests/e2e/. CurrentPID() is an RLock-guarded read of the same field, so ""-when-unstarted semantics and the existing strconv.Atoi error path are unchanged; verified against upstream source rather than assumed.

The assertion, restored. The spec now Run()s a real /bin/sleep 300 child and after the stop asserts both that Done() closes (which only happens once go-processmanager has Wait()ed on the child) and that the PID is gone via a kernel Signal(0) probe.

The Signal(0) probe is deliberate and worth a reviewer's attention: IsAlive() and every pidfile-based helper are useless as termination assertions here, because Stop() deletes the pidfile in release(). They would report "not alive" even if no signal had ever been sent, so a test built on them would pass against a completely broken stop.

The new assertions were falsified before being trusted — pointing the supervisor at a decoy process made the spec fail on the Done() assertion, confirming it has teeth. Reverted immediately after.

Verification with a real process started under -race:

==> [1/3] Go lifecycle and busy-accounting conformance with -race
ok  pkg/model  2.910s
ok  pkg/grpc   2.145s
ok  core/services/worker  6.550s
==> [2/3] ok  core/services/nodes  1.219s
==> [3/3] FizzBee ... PASSED
==> model-lifecycle-conformance OK (Go + FizzBee)

So the upstream fix holds under LocalAI's own fail-closed race gate, not only in its own suite.

make lint 0 issues; coverage gate OK at 53.4% (baseline 48.5%, untouched).

Note for anyone running the gate on a shared box: core/http needs LOCALAI_TEST_HTTP_PORT set when something else holds :9090, otherwise it burns all five flake-retries on bind: address already in use and fails on an environmental collision rather than a real one.