Engine management stays responsive and shuts down reliably

SECURITY.md

@@ -130,6 +130,12 @@ with identity and trust material. It does not automatically protect loopback
 HTTP, discovery, broker-managed node-info HTTP, OTLP export, Electron IPC,
 third-party engine APIs, or traffic outside PAIR.
 
+Local engine `bind` overrides require loopback by default. The explicit node
+setting `engine_allow_lan_bind` permits non-loopback overrides and is read live,
+failing closed if unavailable. It does not authenticate the engine API; remote
+starts stay loopback-only. Disabling it does not rebind existing listeners, and
+user-authored manifest commands remain the operator's responsibility.
+
 Engine installation executes downloaded vendor software; checksum verification
 depends on the engine manifest and is not universally required. Catalogs,
 updates, model downloads, Docker image pulls, engines, and applications can

desktop/docs/service-contract-exceptions.json

@@ -1,6 +1,8 @@
 {
     "$comment": "Intentionally unintegrated JSON-RPC methods for desktop ↔ services contract checks. Each ignoredMethods entry needs a current technical reason. Edit by hand; run npm run service-contracts:write to refresh docs/services-api.md after services/ changes.",
     "ignoredMethods": {
+        "engine:settings-request": "Worker-to-broker internal live permission read. Only settings/get-engine-allow-lan-bind is allowed; the broker returns a correlated engine:settings-reply directly to the originating worker, never the renderer.",
+        "engine:settings-reply": "Broker-to-worker internal response for the live LAN-bind permission read. Missing, malformed, failed or timed-out values deny non-loopback local start overrides.",
         "schedule:priority": "Broker-internal. nvpair-job-scheduler emits a per-engine priority snapshot ({engine, nodes, ranks?} — ordered node ids plus each node's pending count) to nvpair-ui-broker, which forwards it to ollama-proxy and lmstudio-proxy via node/set-priority; each proxy then layers its own optimistic burst reservations on that baseline. PAIR consumes the routing result rather than this notification, and must not mirror the pending/reservation accounting.",
         "engine:restore-enabled": "Broker-internal startup restoration. nvpair-ui-broker emits engine:restore-enabled directly to its supervised engine-manager after the managed Ollama port gate and on manager respawn; it is not a renderer/UI notification.",
         "proxy:ready": "Consumed, not missing: the broker relays it and normalizeBrokerProxy (modular-supervisor.ts) strips the `proxy:` prefix, so the bridge handles the de-prefixed `ready` (sets proxyPort). The literal `proxy:ready` is intentionally absent from our TS — extractor limitation, not a gap.",

desktop/docs/services-api.md

@@ -31,9 +31,11 @@
 - ⚠️ nvpair-job-scheduler → scheduler:tick
 - ⚠️ nvpair-node-info → n/a
 - ⚠️ nvpair-node-settings → settings/get-cluster-auto-sync
+- ⚠️ nvpair-node-settings → settings/get-engine-allow-lan-bind
 - ⚠️ nvpair-node-settings → settings/get-force-ports
 - ⚠️ nvpair-node-settings → settings/get-telemetry-config
 - ⚠️ nvpair-node-settings → settings/set-cluster-auto-sync
+- ⚠️ nvpair-node-settings → settings/set-engine-allow-lan-bind
 - ⚠️ nvpair-node-settings → settings/set-force-ports
 - ⚠️ nvpair-ui-broker → discovery:unsubscribe
 - ⚠️ nvpair-ui-broker → engine:set-reserved-port
@@ -42,6 +44,7 @@
 - ⚠️ nvpair-ui-broker → metrics:unsubscribe
 - ⚠️ nvpair-ui-broker → proxy:get-status
 - ⚠️ nvpair-ui-broker → proxy:unsubscribe
+- ⚠️ nvpair-ui-broker → settings/get-engine-allow-lan-bind
 - ⚠️ nvpair-ui-broker → workloads:unsubscribe
 - ⚠️ ollama-proxy → node/selected
 - ⚠️ ollama-proxy → node/set-local-backend
@@ -214,11 +217,13 @@
 | `settings/get-cluster-auto-sync` | request (we call) | ⚠️ not called |
 | `settings/get-cluster-friendly-name` | request (we call) | ✅ yes |
 | `settings/get-cluster-id` | request (we call) | ✅ yes |
+| `settings/get-engine-allow-lan-bind` | request (we call) | ⚠️ not called |
 | `settings/get-force-ports` | request (we call) | ⚠️ not called |
 | `settings/get-telemetry-config` | request (we call) | ⚠️ not called |
 | `settings/set-cluster-auto-sync` | request (we call) | ⚠️ not called |
 | `settings/set-cluster-friendly-name` | request (we call) | ✅ yes |
 | `settings/set-cluster-id` | request (we call) | ✅ yes |
+| `settings/set-engine-allow-lan-bind` | request (we call) | ⚠️ not called |
 | `settings/set-force-ports` | request (we call) | ⚠️ not called |
 | `settings/set-telemetry-config` | request (we call) | ✅ yes |
 
@@ -296,6 +301,7 @@ Client surface: requests are sent to the broker; notifications are consumed from
 | `broker:epoch` | notification (we consume) | ✅ yes |
 | `discovery:nodes-changed` | notification (we consume) | ✅ yes |
 | `engine:restore-enabled` | notification (we consume) | ➖ ignored |
+| `engine:settings-reply` | notification (we consume) | ➖ ignored |
 | `errors:clear` | notification (we consume) | ✅ yes |
 | `errors:report` | notification (we consume) | ✅ yes |
 | `errors:update` | notification (we consume) | ✅ yes |
@@ -334,6 +340,7 @@ Client surface: requests are sent to the broker; notifications are consumed from
 | `proxy:subscribe` | request (we call) | ✅ yes |
 | `proxy:unsubscribe` | request (we call) | ⚠️ not called |
 | `ready` | request (we call) | ✅ yes |
+| `settings/get-engine-allow-lan-bind` | request (we call) | ⚠️ not called |
 | `telemetry/get-status` | request (we call) | ✅ yes |
 | `telemetry/purge-storage` | request (we call) | ✅ yes |
 | `telemetry/start` | request (we call) | ✅ yes |

desktop/src/electron/service-bridge/modular-state.ts

@@ -291,6 +291,16 @@ function numberValue(value: JsonValue | undefined): number {
     return typeof value === 'number' ? value : 0
 }
 
+/**
+ * An install frame's percent, or undefined when the step is indeterminate. The
+ * engine-manager omits percent (or sends 0) for steps it cannot measure; each
+ * frame replaces the last, so a missing percent must not carry forward.
+ */
+function measuredPercent(value: JsonValue | undefined): number | undefined {
+    const pct = numberValue(value)
+    return pct > 0 ? pct : undefined
+}
+
 function booleanValue(value: JsonValue | undefined): boolean {
     return typeof value === 'boolean' ? value : false
 }
@@ -2284,11 +2294,10 @@ class ModularBridgeState {
                 ? engineProgressKey({ nodeId, engineType, operation: 'pull', model })
                 : ''
         const existingPull = pullKey ? this.activePulls.get(pullKey) : undefined
-        const framePercent = numberValue(obj.percent)
         const percent =
             operation === 'pull'
-                ? mergePullProgressPercent(framePercent, existingPull?.percent)
-                : framePercent
+                ? mergePullProgressPercent(numberValue(obj.percent), existingPull?.percent)
+                : measuredPercent(obj.percent)
         const progress: EngineProgress = {
             engineType,
             nodeId,
@@ -2869,7 +2878,7 @@ class ModularBridgeState {
             nodeName: node?.name ?? nodeId,
             operation: 'install',
             status: stage,
-            percent: numberValue(obj.percent)
+            percent: measuredPercent(obj.percent)
         }
         emitBridgePush('engines:progress-changed', progress)
     }

desktop/tests/modular/install-state.test.ts

@@ -6,7 +6,7 @@ import { afterEach, describe, expect, it, vi } from 'vitest'
 vi.mock('electron', () => ({ BrowserWindow: { getAllWindows: () => [] } }))
 vi.mock('@/electron/window', () => ({ createOverviewWindow: vi.fn() }))
 
-import type { EngineProcessStatus } from '@/shared/types/engines'
+import type { EngineProcessStatus, EngineProgress } from '@/shared/types/engines'
 import { subscribePush } from '@/electron/service-bridge/push-bus'
 import { getModularBridgeState } from '@/electron/service-bridge/modular-state'
 
@@ -46,7 +46,7 @@ describe('LM Studio install state', () => {
         vi.advanceTimersByTime(120_000)
         expect(statuses.at(-1)).toBe('installing')
 
-        state.applyEngineManagerProgress({ engine: 'lmstudio', stage: 'installing', percent: 75 })
+        state.applyEngineManagerProgress({ engine: 'lmstudio', stage: 'installing' })
         vi.advanceTimersByTime(30 * 60_000 - 1)
         expect(statuses.at(-1)).toBe('installing')
         vi.advanceTimersByTime(1)
@@ -57,3 +57,47 @@ describe('LM Studio install state', () => {
         expect(statuses.at(-1)).toBe('not-installed')
     })
 })
+
+describe('install progress percent', () => {
+    function captureInstallProgress(): EngineProgress[] {
+        const frames: EngineProgress[] = []
+        unsubscribe = subscribePush(event => {
+            if (
+                event.channel === 'engines:progress-changed' &&
+                event.payload.operation === 'install'
+            ) {
+                frames.push(event.payload)
+            }
+        })
+        return frames
+    }
+
+    it('drops a local install to indeterminate after a measured download', () => {
+        const state = getModularBridgeState()
+        const frames = captureInstallProgress()
+        state.setSelfId('local-node')
+
+        state.applyEngineManagerProgress({ engine: 'lmstudio', stage: 'downloading', percent: 42 })
+        expect(frames.at(-1)?.percent).toBe(42)
+
+        state.applyEngineManagerProgress({ engine: 'lmstudio', stage: 'downloading', percent: 100 })
+        state.applyEngineManagerProgress({ engine: 'lmstudio', stage: 'installing' })
+        expect(frames.at(-1)?.status).toBe('installing')
+        expect(frames.at(-1)?.percent).toBeUndefined()
+    })
+
+    it('drops a remote install to indeterminate after a measured download', () => {
+        const state = getModularBridgeState()
+        const frames = captureInstallProgress()
+        const frame = { node: 'remote-node', engine: 'ollama', op: 'install' }
+
+        state.applyRemoteEngineProgress({ ...frame, stage: 'downloading', percent: 100 })
+        expect(frames.at(-1)?.percent).toBe(100)
+
+        state.applyRemoteEngineProgress({ ...frame, stage: 'installing' })
+        expect(frames.at(-1)?.status).toBe('installing')
+        expect(frames.at(-1)?.percent).toBeUndefined()
+
+        state.applyRemoteEngineProgress({ ...frame, stage: 'failed', percent: -1 })
+    })
+})

services/nvpair-engine-manager/README.md

@@ -213,7 +213,7 @@ Requests (caller → service):
 | `engine:remote-install` | `{ node, engine, start? }` | `{ opId, status: EngineStatus }` after the remote install (live progress via `engine:remote-progress`) |
 | `engine:remote-install-tool` | `{ node, tool }` | `{ opId, result: { tool, installed: true } }`, or the remote tool's typed error |
 | `engine:remote-pull-model` | `{ node, engine, model?, params? }` | `{ opId, result }` after a successful remote pull (live progress via `engine:remote-progress`); engine-level failures, including HTTP 200 streams containing `error`, return a JSON-RPC error |
-| `engine:remote-start` | `{ node, engine, port? }` | `EngineStatus` from the remote node (always the manifest's `runtime.bind`; no per-call bind override on the remote path) |
+| `engine:remote-start` | `{ node, engine, port? }` | `EngineStatus` from the remote node (always loopback `127.0.0.1`; no per-call bind override on the remote path) |
 | `engine:remote-stop` | `{ node, engine }` | `EngineStatus` from the remote node |
 | `shutdown` | — | `null` |
 | `log/set-level` | `{ level }` | `{ level }` |
@@ -242,6 +242,51 @@ a successful command must leave the tool detectable.
 | `-32011` | `unsupported-platform`: the node is not `darwin/arm64`. |
 | `-32012` | `install-failed`: the install command failed or macmon was not detected afterwards. Command failures include only the last 20 stderr lines, capped to the last 4096 bytes; stdout is discarded. |
 
+### Startup cancellation
+
+A command-mode control CLI can launch its daemon before the CLI exits. Failed
+or canceled starts run the bounded official stop command if a CLI actually ran,
+including nonzero exits, before releasing the lifecycle lock. A command that
+could not spawn does not authorize cleanup. Shutdown cancellation therefore
+cannot skip cleanup merely because readiness had not begun.
+
+### Shutdown sweep
+
+`engine:prepare-shutdown` and the EOF/transport-exit path join one concurrent
+engine sweep per executor. Every caller waits for it to finish. Later calls
+retry only process-handle or command-stop failures that can still be resolved;
+a declined foreign listener is not repeatedly probed. Shutdown rejects new starts
+and never changes saved ON/OFF intent. Engines still stop concurrently.
+
+### Local bind permission
+
+A local `bind` override must be an IP address on loopback unless the operator
+explicitly enables the persisted node setting `engine_allow_lan_bind` (default
+false). Engine-manager reads it live through the broker on each non-loopback
+override; absent settings workers, read errors, invalid replies and a two-second
+timeout all deny permission. Revoking it affects subsequent starts, not listeners
+already running. Remote starts always use `127.0.0.1`, regardless of this setting.
+This opt-in does not add authentication to third-party engine APIs. It governs
+per-call overrides, not user-authored manifest launch commands.
+
+Internal notifications `engine:settings-request{id, method}` (only
+`settings/get-engine-allow-lan-bind`) and
+`engine:settings-reply{id, result, error?}` carry the correlated read between
+worker and broker; neither is a client subscription event.
+
+### Passive status and model deadlines
+
+Status and get-installed share bounded presence probes without holding the
+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.
+
+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
+than a successful partial or empty list; peers retain their last-good inventory.
+The local `engine:models` result remains best-effort with the same payload.
+
 ### Effective capabilities
 
 `EngineStatus` = `{ engine, display_name, installed, running, healthy, port?, capabilities }`.
@@ -273,18 +318,24 @@ Notifications (service → caller): `engine:ready{version}`,
 loaded (in-memory) models changes (explicit load/unload, JIT auto-load, or
 TTL/idle eviction); `models` is the full `engine:models` shape (incl.
 `loadedByEngine`) so a consumer swaps its whole snapshot,
-`engine:install-progress{engine, stage, percent}`,
-`engine:pull-progress{engine, op, stage, percent, message}` (live progress for a
+`engine:install-progress{engine, stage, percent?, error?}`,
+`engine:pull-progress{engine, op, stage, percent?, message}` (live progress for a
 local model pull driven via `engine:action{action:"pull_model"}` — the local
 counterpart of `engine:remote-progress`; frames are coalesced to changes in
 stage/percent, the engine's terminal success surfaces as `stage:"success"`, and
 a failed pull emits a terminal `stage:"error", percent:-1, message` frame so a
 UI converges even if its synchronous call already timed out),
-`engine:remote-progress{opId, node, engine, op, stage, percent, message}`
+`engine:remote-progress{opId, node, engine, op, stage, percent?, message}`
 (relayed live progress for a remote install/pull), and — for the error
 pipeline — `errors:report` / `errors:clear` (consumed by `nvpair-errors`
 via the broker; see below).
 
+Progress `percent` is optional: unmeasured stages omit it (zero internally),
+positive values report measured download progress or terminal completion, and
+negative values are failure sentinels. Verification, vendor install commands,
+and disk-image copies report stages without invented percentages. Clients must
+replace the previous install percentage with an indeterminate stage when absent.
+
 For `engine:action` with `action: "pull_model"`, a streaming engine-level
 failure (including an HTTP 200 NDJSON `{"error":"…"}` frame) returns a
 JSON-RPC error, not a successful result containing an error body. The peer's

services/nvpair-engine-manager/actions.go

@@ -64,8 +64,8 @@ func (e *Executor) Action(ctx context.Context, engine, action string, params jso
 //     model must not rewrite the engine's saved ON/OFF intent. doStop/doStart
 //     are the intent-neutral pair (StopAll uses them for the same reason).
 func (e *Executor) restartAfterAction(ctx context.Context, st *engineState, engine, action string) error {
-	st.opMu.Lock()
-	defer st.opMu.Unlock()
+	st.lockOperation()
+	defer st.unlockOperation()
 	st.mu.Lock()
 	running := st.running
 	st.mu.Unlock()