diff --git a/frontend/src-tauri/src/lib.rs b/frontend/src-tauri/src/lib.rs index 679ed147..ebae60e9 100644 --- a/frontend/src-tauri/src/lib.rs +++ b/frontend/src-tauri/src/lib.rs @@ -1540,19 +1540,89 @@ async fn fetch_models(api_url: String) -> Result { #[tauri::command] async fn run_jarvis_command(args: Vec) -> Result { - let mut cmd_args = vec!["run".to_string(), "jarvis".to_string()]; - cmd_args.extend(args); let uv_bin = resolve_bin("uv"); - let output = tokio::process::Command::new(&uv_bin) - .args(&cmd_args) - .output() - .await - .map_err(|e| format!("Failed to launch jarvis: {}", e))?; - if output.status.success() { - Ok(String::from_utf8_lossy(&output.stdout).to_string()) - } else { - Err(String::from_utf8_lossy(&output.stderr).to_string()) + let mut cmd_args = vec!["run".to_string(), "jarvis".to_string()]; + cmd_args.extend(args.iter().cloned()); + + let mut cmd = tokio::process::Command::new(&uv_bin); + cmd.args(&cmd_args); + // Run from the project root so `uv run jarvis` resolves the OpenJarvis + // project regardless of the app's launch cwd. In a packaged install the + // cwd isn't the checkout, so without this `jarvis` isn't found and the + // backend never starts — the UI then shows "Failed to get response" + // (see #531). + if let Some(ref root) = find_project_root() { + cmd.current_dir(root); + } + + let is_serve = args.first().map(|a| a.as_str() == "serve").unwrap_or(false); + + if !is_serve { + // Short-lived command (e.g. `stop`, `status`): wait for it and return + // its captured output. + let output = cmd + .output() + .await + .map_err(|e| format!("Failed to launch jarvis: {}", e))?; + return if output.status.success() { + Ok(String::from_utf8_lossy(&output.stdout).to_string()) + } else { + Err(String::from_utf8_lossy(&output.stderr).to_string()) + }; + } + + // `jarvis serve` is a long-running server that never exits. The old code + // used `.output()`, which waits for the process to exit and so hung this + // command forever — the "Start" button never resolved (#531). Spawn it + // detached instead, drain stderr (a full 4 KB Windows pipe can otherwise + // stall the child mid-startup, #309), and poll /health for readiness. + cmd.stdout(std::process::Stdio::null()) + .stderr(std::process::Stdio::piped()); + let mut child = cmd + .spawn() + .map_err(|e| format!("Failed to launch jarvis serve: {}", e))?; + + let tail: StderrTail = Arc::new(Mutex::new(Vec::new())); + if let Some(stderr) = child.stderr.take() { + spawn_jarvis_stderr_drainer(stderr, tail.clone()); + } + + let client = reqwest::Client::builder() + .timeout(Duration::from_secs(2)) + .build() + .map_err(|e| format!("Failed to build HTTP client: {}", e))?; + let url = format!("http://127.0.0.1:{}/health", JARVIS_PORT); + let deadline = tokio::time::Instant::now() + Duration::from_secs(120); + + loop { + // Surface an early crash (bad venv, missing Rust ext, etc.) right away + // instead of waiting out the full readiness timeout. + if let Ok(Some(status)) = child.try_wait() { + let stderr = String::from_utf8_lossy(tail.lock().await.as_slice()).into_owned(); + return Err(format!( + "jarvis serve exited (code {:?}) before becoming healthy:\n{}", + status.code(), + stderr.trim() + )); + } + if let Ok(resp) = client.get(&url).send().await { + if resp.status().is_success() { + // Leave the server running (the Child is detached on drop — + // kill_on_drop defaults to false); `stop` tears it down. + return Ok(format!( + "jarvis serve is ready on http://127.0.0.1:{}", + JARVIS_PORT + )); + } + } + if tokio::time::Instant::now() >= deadline { + return Err(format!( + "jarvis serve did not become healthy on port {} within 120s.", + JARVIS_PORT + )); + } + tokio::time::sleep(Duration::from_millis(500)).await; } } diff --git a/src/openjarvis/cli/ask.py b/src/openjarvis/cli/ask.py index c0c9ad8a..91804878 100644 --- a/src/openjarvis/cli/ask.py +++ b/src/openjarvis/cli/ask.py @@ -714,7 +714,13 @@ def ask( register_builtin_models() effective_engine_key = engine_key or config.intelligence.preferred_engine or None - resolved = get_engine(config, effective_engine_key) + # Pass the model we intend to run so engine selection can skip an engine + # that can't actually serve it (e.g. the cloud fallback when the local + # engine is down but only a non-OpenAI key is set — see #532). This is the + # -m flag or the configured default; when neither is set we leave it None + # and a model is chosen per-engine below. + selection_model = model_name or config.intelligence.default_model or None + resolved = get_engine(config, effective_engine_key, model=selection_model) if resolved is None: console.print( "[red bold]No inference engine available.[/red bold]\n\n" diff --git a/src/openjarvis/cli/serve.py b/src/openjarvis/cli/serve.py index 0f1bf258..ae59c7db 100644 --- a/src/openjarvis/cli/serve.py +++ b/src/openjarvis/cli/serve.py @@ -146,7 +146,13 @@ def serve( except Exception as exc: logger.debug("Telemetry store init failed: %s", exc) - resolved = get_engine(config, engine_key) + # Select with the model we'll actually serve so an engine that can't + # serve it (e.g. the cloud fallback without the matching provider key) is + # skipped rather than chosen and failing per-request later (see #532). + selection_model = ( + model_name or config.server.model or config.intelligence.default_model or None + ) + resolved = get_engine(config, engine_key, model=selection_model) if resolved is None: console.print( "[red bold]No inference engine available.[/red bold]\n\n" diff --git a/src/openjarvis/engine/_discovery.py b/src/openjarvis/engine/_discovery.py index 6de6c1be..d1b18bbd 100644 --- a/src/openjarvis/engine/_discovery.py +++ b/src/openjarvis/engine/_discovery.py @@ -156,12 +156,26 @@ def discover_models( def get_engine( - config: JarvisConfig, engine_key: str | None = None + config: JarvisConfig, + engine_key: str | None = None, + model: str | None = None, ) -> Tuple[str, InferenceEngine] | None: """Get a specific engine by key, or the default with fallback. + When *model* is given, an engine is selected only if it can actually + serve that model (``engine.can_serve(model)``). This stops the cloud + fallback from being chosen — when the local engine is down — for a model + whose provider client is missing, which otherwise surfaces as a confusing + "OpenAI client not available" instead of a helpful "start your local + engine" message (see #532). When *model* is ``None`` selection stays + model-agnostic (unchanged behaviour). + Returns ``(key, engine_instance)`` or ``None`` if no engine is available. """ + + def _usable(engine: InferenceEngine) -> bool: + return engine.health() and (model is None or engine.can_serve(model)) + # Build an ordered list of keys to try, then fall back to full discovery. keys_to_try: list[str] = [] if engine_key: @@ -176,14 +190,16 @@ def get_engine( continue try: engine = _make_engine(key, config) - if engine.health(): + if _usable(engine): return (key, engine) except Exception as exc: logger.debug("Engine %r health check failed: %s", key, exc) - # Fallback to any healthy engine - healthy = discover_engines(config) - return healthy[0] if healthy else None + # Fallback to the first healthy engine that can serve the model. + for key, engine in discover_engines(config): + if model is None or engine.can_serve(model): + return (key, engine) + return None __all__ = ["discover_engines", "discover_models", "get_engine"] diff --git a/src/openjarvis/engine/_stubs.py b/src/openjarvis/engine/_stubs.py index c0629502..cd7dc0d4 100644 --- a/src/openjarvis/engine/_stubs.py +++ b/src/openjarvis/engine/_stubs.py @@ -119,6 +119,17 @@ class InferenceEngine(ABC): def health(self) -> bool: """Return ``True`` when the engine is reachable and healthy.""" + def can_serve(self, model: str) -> bool: + """Return ``True`` if this engine can serve *model*. + + Defaults to ``True``: local engines accept any model id (whether a + specific model is *installed* is a separate concern from engine + selection). Engines that multiplex provider-specific clients (e.g. + the cloud engine) override this so selection can skip an engine whose + client for the model's provider isn't configured (see #532). + """ + return True + def close(self) -> None: """Release resources (HTTP clients, connections, threads, etc.).""" diff --git a/src/openjarvis/engine/cloud.py b/src/openjarvis/engine/cloud.py index 88c45bc6..ea67c258 100644 --- a/src/openjarvis/engine/cloud.py +++ b/src/openjarvis/engine/cloud.py @@ -1477,6 +1477,34 @@ class CloudEngine(InferenceEngine): models.extend(_CODEX_MODELS) return models + def _client_for_model(self, model: str) -> Any: + """Return the provider client ``generate``/``stream`` will dispatch to + for *model* (mirrors the routing in those methods).""" + if _is_codex_model(model): + return self._codex_client + if _is_openrouter_model(model): + return self._openrouter_client + if _is_minimax_model(model): + return self._minimax_client + if _is_anthropic_model(model): + return self._anthropic_client + if _is_google_model(model): + return self._google_client + return self._openai_client + + def can_serve(self, model: str) -> bool: + """Return ``True`` only if the provider client for *model* exists. + + ``health()`` is ``True`` whenever *any* provider client is configured, + but a request for, say, a ``gpt-*`` model still needs the OpenAI + client specifically. Without this check the cloud engine gets picked + as a fallback (when the local engine is down) for a model it can't + serve, then dies at call time with " client not available" + instead of the user getting a helpful "start your local engine" + message (see #532). + """ + return self._client_for_model(model) is not None + def health(self) -> bool: return ( self._openai_client is not None