From 50993dfa4dae1b7da66c8583fa79c853a8ba27cd Mon Sep 17 00:00:00 2001 From: Jon Saad-Falcon <41205309+jonsaadfalcon@users.noreply.github.com> Date: Thu, 11 Jun 2026 13:49:37 -0700 Subject: [PATCH] fix(evals): honor --base-url/--api-key for first-party eval backends (#535) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `jarvis eval run --base-url ... --api-key ...` was silently dropped for jarvis-direct/jarvis-agent (_build_backend only forwarded the flags to hermes/openclaw) and ignored by terminalbench-native, which hardcoded api_base="http://localhost:8000/v1". Worse, with --base-url set the engine-discovery fallback silently substituted ANY healthy local engine (observed: requested vllm + healthy endpoint at --base-url, got OllamaEngine@localhost:11434 — the requested URL was never contacted). Changes: - _OpenAICompatibleEngine gains an api_key param (Bearer Authorization header on the httpx client; {ENGINE_ID}_API_KEY env fallback with hyphen-sanitized names; no header when unset). - New non-registered OpenAICompatEngine + normalize_openai_base_url() (strips a single literal trailing "/v1" so request paths don't double). - SystemBuilder.engine_instance() injects a pre-built engine; build() health-checks it and fails loudly naming the host instead of falling back to discovery. Discovery substitution after an explicit -e key now logs a warning. - JarvisDirectBackend/JarvisAgentBackend accept base_url/api_key; on base_url they pin an OpenAICompatEngine to that endpoint with a fail-fast pre-flight (actionable error naming the URL and probe). - _build_backend forwards base_url/api_key to first-party backends on the CLI path; _run_terminalbench_native receives --base-url as api_base (single /v1 suffix) and exports OPENAI_API_KEY around the in-process harness run (terminus-2 routes via LiteLLM). - Suite TOML [backend.external] stays scoped to hermes/openclaw (suite_mode=True in the suite drivers) — first-party suite semantics are explicitly deferred. The config-host path is untouched. - Help text updated on both CLI surfaces; KNOWN_BACKENDS now lists hermes/openclaw/terminalbench-native. Fixes the eval-CLI endpoint gap reported by the downstream team. Co-authored-by: Claude Opus 4.8 (1M context) --- src/openjarvis/cli/compose_cmd.py | 2 +- src/openjarvis/cli/eval_cmd.py | 24 ++- src/openjarvis/engine/_openai_compat.py | 27 ++- .../engine/openai_compat_engines.py | 35 +++- .../evals/backends/_endpoint_util.py | 52 +++++ src/openjarvis/evals/backends/jarvis_agent.py | 14 +- .../evals/backends/jarvis_direct.py | 14 +- src/openjarvis/evals/cli.py | 141 ++++++++++--- src/openjarvis/evals/tests/test_backends.py | 148 ++++++++++++++ .../evals/tests/test_cli_endpoint.py | 190 ++++++++++++++++++ src/openjarvis/system/builder.py | 46 ++++- tests/engine/test_openai_compat_api_key.py | 109 ++++++++++ tests/sdk/test_system.py | 81 +++++++- 13 files changed, 838 insertions(+), 45 deletions(-) create mode 100644 src/openjarvis/evals/backends/_endpoint_util.py create mode 100644 src/openjarvis/evals/tests/test_cli_endpoint.py create mode 100644 tests/engine/test_openai_compat_api_key.py diff --git a/src/openjarvis/cli/compose_cmd.py b/src/openjarvis/cli/compose_cmd.py index e7af878c..4995fa50 100644 --- a/src/openjarvis/cli/compose_cmd.py +++ b/src/openjarvis/cli/compose_cmd.py @@ -332,7 +332,7 @@ def compose_bench( for i, rc in enumerate(run_configs, 1): console.print(f"\n[bold]Run {i}/{len(run_configs)}:[/bold] {rc.benchmark}") try: - summary = _run_single(rc, console=console) + summary = _run_single(rc, console=console, suite_mode=True) results_table.add_row( rc.benchmark, f"{summary.accuracy:.4f}", diff --git a/src/openjarvis/cli/eval_cmd.py b/src/openjarvis/cli/eval_cmd.py index f2e9cde6..ed84af4a 100644 --- a/src/openjarvis/cli/eval_cmd.py +++ b/src/openjarvis/cli/eval_cmd.py @@ -61,6 +61,12 @@ KNOWN_BENCHMARKS = { KNOWN_BACKENDS = { "jarvis-direct": "Engine-level inference (local or cloud)", "jarvis-agent": "Agent-level inference with tool calling", + "hermes": "Real Hermes Agent (Nous Research) via subprocess", + "openclaw": "Real OpenClaw via Node subprocess", + "terminalbench-native": ( + "TerminalBench V2.1 via terminal-bench Harness " + "(selected with -b terminalbench-native)" + ), } @@ -146,7 +152,9 @@ def eval_list() -> None: "base_url", default=None, help=( - "OpenAI-compat endpoint URL for hermes/openclaw backends " + "OpenAI-compatible endpoint for the model under eval. Required for " + "hermes/openclaw; for jarvis-direct/jarvis-agent/terminalbench-native " + "it bypasses engine discovery and targets this URL directly " "(env: JARVIS_BACKEND_BASE_URL)." ), ) @@ -154,7 +162,11 @@ def eval_list() -> None: "--api-key", "api_key", default=None, - help=("API key for the hermes/openclaw endpoint (env: JARVIS_BACKEND_API_KEY)."), + help=( + "API key for the --base-url endpoint, sent as a Bearer token. " + "Required for hermes/openclaw; optional for first-party backends " + "(env: JARVIS_BACKEND_API_KEY)." + ), ) @click.option( "--agent", @@ -347,7 +359,7 @@ def eval_run( f"{rc.benchmark} / {rc.model}" ) try: - summary = _run_single(rc, console=console) + summary = _run_single(rc, console=console, suite_mode=True) console.print( f" [green]{summary.accuracy:.4f}[/green] " f"({summary.correct}/{summary.scored_samples})" @@ -399,8 +411,10 @@ def eval_run( sheets_spreadsheet_id=sheets_spreadsheet_id, sheets_worksheet=sheets_worksheet, sheets_credentials_path=sheets_credentials_path, - # Spec §6.2 — for hermes/openclaw external backends. Falls back to env vars - # so users can also set JARVIS_BACKEND_BASE_URL/JARVIS_BACKEND_API_KEY. + # OpenAI-compatible endpoint for the model under eval. Required for + # hermes/openclaw (Spec §6.2); honored by first-party backends too on + # this CLI path. Falls back to env vars so users can also set + # JARVIS_BACKEND_BASE_URL/JARVIS_BACKEND_API_KEY. base_url=base_url or os.environ.get("JARVIS_BACKEND_BASE_URL"), api_key=api_key or os.environ.get("JARVIS_BACKEND_API_KEY"), ) diff --git a/src/openjarvis/engine/_openai_compat.py b/src/openjarvis/engine/_openai_compat.py index edbf6d44..5f767478 100644 --- a/src/openjarvis/engine/_openai_compat.py +++ b/src/openjarvis/engine/_openai_compat.py @@ -28,12 +28,31 @@ class _OpenAICompatibleEngine(InferenceEngine): _default_host: str = "http://localhost:8000" _api_prefix: str = "/v1" - def __init__(self, host: str | None = None, *, timeout: float = 600.0) -> None: + def __init__( + self, + host: str | None = None, + *, + api_key: str | None = None, + timeout: float = 600.0, + ) -> None: import os - env_key = f"{self.engine_id.upper()}_HOST" - self._host = (host or os.environ.get(env_key) or self._default_host).rstrip("/") - self._client = httpx.Client(base_url=self._host, timeout=timeout) + # Sanitize the engine id for env-var lookup ("openai-compat" -> + # "OPENAI_COMPAT_..."); shells cannot set hyphenated variable names. + env_prefix = self.engine_id.upper().replace("-", "_") + self._host = ( + host or os.environ.get(f"{env_prefix}_HOST") or self._default_host + ).rstrip("/") + # Bearer auth for endpoints started with e.g. ``vllm serve --api-key``. + # Setting it on the client covers generate/stream/stream_full/ + # list_models/health alike; ``None`` keeps requests header-free. + self._api_key = api_key or os.environ.get(f"{env_prefix}_API_KEY") or None + headers = ( + {"Authorization": f"Bearer {self._api_key}"} if self._api_key else None + ) + self._client = httpx.Client( + base_url=self._host, timeout=timeout, headers=headers + ) # -- InferenceEngine interface ------------------------------------------ diff --git a/src/openjarvis/engine/openai_compat_engines.py b/src/openjarvis/engine/openai_compat_engines.py index 6d191f3d..1c2d178e 100644 --- a/src/openjarvis/engine/openai_compat_engines.py +++ b/src/openjarvis/engine/openai_compat_engines.py @@ -1,5 +1,7 @@ """Data-driven registration of OpenAI-compatible inference engines.""" +from __future__ import annotations + from openjarvis.core.registry import EngineRegistry from openjarvis.engine._openai_compat import _OpenAICompatibleEngine @@ -25,4 +27,35 @@ for _key, (_cls_name, _default_host, _api_prefix) in _ENGINES.items(): EngineRegistry.register(_key)(_cls) globals()[_cls_name] = _cls -__all__ = [name for name, _, _ in _ENGINES.values()] + +def normalize_openai_base_url(url: str) -> str: + """Strip a single trailing ``/v1`` segment from a user-supplied base URL. + + Users habitually pass ``http://host:8000/v1`` (the full OpenAI-compatible + prefix); the engine's ``_api_prefix`` re-appends ``/v1`` to every request + path, so a trailing copy would double up as ``/v1/v1``. Only a literal + trailing ``/v1`` is stripped — proxy/gateway path prefixes are preserved. + """ + base = url.rstrip("/") + if base.endswith("/v1"): + base = base[: -len("/v1")] + return base + + +class OpenAICompatEngine(_OpenAICompatibleEngine): + """Generic engine for an explicitly-provided OpenAI-compatible endpoint. + + Deliberately NOT registered in ``EngineRegistry``: it is only ever + constructed with an explicit host (e.g. ``jarvis eval --base-url``), so + registering it would just add a useless localhost discovery probe and + interact with the per-test registry wipe. + """ + + engine_id = "openai-compat" + _api_prefix = "/v1" + + +__all__ = [name for name, _, _ in _ENGINES.values()] + [ + "OpenAICompatEngine", + "normalize_openai_base_url", +] diff --git a/src/openjarvis/evals/backends/_endpoint_util.py b/src/openjarvis/evals/backends/_endpoint_util.py new file mode 100644 index 00000000..cf60c495 --- /dev/null +++ b/src/openjarvis/evals/backends/_endpoint_util.py @@ -0,0 +1,52 @@ +"""Shared helper for targeting an explicit OpenAI-compatible endpoint. + +Used by the first-party eval backends (jarvis-direct, jarvis-agent) when +``--base-url`` is given: the eval must use exactly that endpoint, with no +silent fallback to whatever other engine discovery happens to find. +""" + +from __future__ import annotations + +import logging +from typing import Optional + +logger = logging.getLogger(__name__) + + +def build_endpoint_engine( + base_url: str, + api_key: Optional[str] = None, + engine_key: Optional[str] = None, +): + """Construct an :class:`OpenAICompatEngine` pinned to ``base_url``. + + Pre-flight health-checks the endpoint and raises a loud, actionable + error when it is unreachable — engine discovery is never consulted. + """ + from openjarvis.engine.openai_compat_engines import ( + OpenAICompatEngine, + normalize_openai_base_url, + ) + + if engine_key: + logger.warning( + "Both an engine key (%r) and base_url (%r) were given; " + "base_url wins — targeting the endpoint directly.", + engine_key, + base_url, + ) + host = normalize_openai_base_url(base_url) + engine = OpenAICompatEngine(host=host, api_key=api_key) + if not engine.health(): + engine.close() + raise RuntimeError( + f"--base-url endpoint not reachable: {base_url} " + f"(GET {host}/v1/models failed). Is an OpenAI-compatible server " + "(e.g. `vllm serve`) running at that address? If it requires " + "authentication (HTTP 401), pass --api-key or set " + "JARVIS_BACKEND_API_KEY." + ) + return engine + + +__all__ = ["build_endpoint_engine"] diff --git a/src/openjarvis/evals/backends/jarvis_agent.py b/src/openjarvis/evals/backends/jarvis_agent.py index 8f876bb4..ef5625da 100644 --- a/src/openjarvis/evals/backends/jarvis_agent.py +++ b/src/openjarvis/evals/backends/jarvis_agent.py @@ -31,6 +31,8 @@ class JarvisAgentBackend(InferenceBackend): max_turns: Optional[int] = None, skills_enabled: bool = True, overlay_dir: Optional[Path] = None, + base_url: Optional[str] = None, + api_key: Optional[str] = None, ) -> None: from openjarvis.system import SystemBuilder @@ -40,7 +42,17 @@ class JarvisAgentBackend(InferenceBackend): self._gpu_metrics = gpu_metrics builder = SystemBuilder() - if engine_key: + if base_url: + # Explicit endpoint targeting (--base-url): pin the eval to + # exactly this OpenAI-compatible endpoint. Fails fast if it is + # unreachable; never falls back to a discovered engine. + from openjarvis.evals.backends._endpoint_util import ( + build_endpoint_engine, + ) + + engine = build_endpoint_engine(base_url, api_key, engine_key) + builder.engine_instance(engine, key=engine_key or "openai-compat") + elif engine_key: builder.engine(engine_key) if model: builder.model(model) diff --git a/src/openjarvis/evals/backends/jarvis_direct.py b/src/openjarvis/evals/backends/jarvis_direct.py index d0195882..b90ba5cd 100644 --- a/src/openjarvis/evals/backends/jarvis_direct.py +++ b/src/openjarvis/evals/backends/jarvis_direct.py @@ -24,6 +24,8 @@ class JarvisDirectBackend(InferenceBackend): engine_key: Optional[str] = None, telemetry: bool = False, gpu_metrics: bool = False, + base_url: Optional[str] = None, + api_key: Optional[str] = None, ) -> None: from openjarvis.system import SystemBuilder @@ -31,7 +33,17 @@ class JarvisDirectBackend(InferenceBackend): self._gpu_metrics = gpu_metrics builder = SystemBuilder() - if engine_key: + if base_url: + # Explicit endpoint targeting (--base-url): pin the eval to + # exactly this OpenAI-compatible endpoint. Fails fast if it is + # unreachable; never falls back to a discovered engine. + from openjarvis.evals.backends._endpoint_util import ( + build_endpoint_engine, + ) + + engine = build_endpoint_engine(base_url, api_key, engine_key) + builder.engine_instance(engine, key=engine_key or "openai-compat") + elif engine_key: builder.engine(engine_key) # Propagate gpu_metrics to the runtime config so SystemBuilder # creates an EnergyMonitor / GpuMonitor for the InstrumentedEngine. diff --git a/src/openjarvis/evals/cli.py b/src/openjarvis/evals/cli.py index 84e804ce..94715db7 100644 --- a/src/openjarvis/evals/cli.py +++ b/src/openjarvis/evals/cli.py @@ -183,14 +183,28 @@ def _build_backend( max_turns: Optional[int] = None, base_url: Optional[str] = None, api_key: Optional[str] = None, + first_party_endpoint: bool = True, ): """Construct the appropriate backend. - For "hermes" and "openclaw" backends, ``base_url`` and ``api_key`` are - REQUIRED — these foreign frameworks need an OpenAI-compatible endpoint - to send model calls to. Pass them via the eval config's - ``[backend.external]`` section or env vars. + ``base_url``/``api_key`` point at the OpenAI-compatible endpoint serving + the model under eval: + + - For "hermes" and "openclaw" they are REQUIRED — these foreign + frameworks always call out to an external endpoint. + - "jarvis-direct" and "jarvis-agent" honor them when + ``first_party_endpoint`` is True (the CLI ``--base-url`` path): the + eval targets exactly that endpoint — no engine-discovery fallback — + and fails fast if it is unreachable. Suite mode passes + ``first_party_endpoint=False`` so the suite TOML's + ``[backend.external]`` section stays scoped to hermes/openclaw + (extending it to first-party backends is explicitly deferred). """ + if not first_party_endpoint: + fp_base_url = fp_api_key = None + else: + fp_base_url, fp_api_key = base_url, api_key + if backend_name == "jarvis-agent": from openjarvis.evals.backends.jarvis_agent import JarvisAgentBackend @@ -202,6 +216,8 @@ def _build_backend( gpu_metrics=gpu_metrics, model=model, max_turns=max_turns, + base_url=fp_base_url, + api_key=fp_api_key, ) elif backend_name == "jarvis-direct": from openjarvis.evals.backends.jarvis_direct import JarvisDirectBackend @@ -210,6 +226,8 @@ def _build_backend( engine_key=engine_key, telemetry=telemetry, gpu_metrics=gpu_metrics, + base_url=fp_base_url, + api_key=fp_api_key, ) elif backend_name == "hermes": from openjarvis.evals.backends.external import HermesBackend @@ -656,8 +674,20 @@ def _build_trackers(config) -> list: return trackers -def _run_terminalbench_native(config, console: Console) -> object: - """Run TerminalBench V2.1 natively via terminal-bench Harness.""" +def _run_terminalbench_native( + config, + console: Console, + *, + base_url: Optional[str] = None, + api_key: Optional[str] = None, +) -> object: + """Run TerminalBench V2.1 natively via terminal-bench Harness. + + ``base_url`` (from ``--base-url`` / JARVIS_BACKEND_BASE_URL) targets an + already-running OpenAI-compatible endpoint; when unset, the legacy local + vLLM default (http://localhost:8000/v1) is used. + """ + from openjarvis.engine.openai_compat_engines import normalize_openai_base_url from openjarvis.evals.backends.terminalbench_native import ( TerminalBenchNativeBackend, ) @@ -668,9 +698,16 @@ def _run_terminalbench_native(config, console: Console) -> object: litellm_model = f"openai/{model}" output_dir = getattr(config, "output_path", None) or "results/terminalbench-native/" + # Normalize to exactly one trailing "/v1" — LiteLLM's api_base wants the + # full OpenAI-compatible prefix, and users pass both forms of the URL. + if base_url: + api_base = normalize_openai_base_url(base_url) + "/v1" + else: + api_base = "http://localhost:8000/v1" + backend = TerminalBenchNativeBackend( model=litellm_model, - api_base="http://localhost:8000/v1", + api_base=api_base, temperature=config.temperature, max_samples=config.max_samples, output_dir=output_dir, @@ -683,9 +720,25 @@ def _run_terminalbench_native(config, console: Console) -> object: model_slug = re.sub(r"[^a-z0-9_-]", "-", model.lower().replace("/", "-")) run_id = f"tb21-{model_slug}" console.print(f" Running TerminalBench V2.1 natively: {model}") + console.print(f" API base: {api_base}") console.print(f" Harness run_id: {run_id}") - results = backend.run_harness(run_id) + if api_key: + # terminus-2 routes model calls through LiteLLM with the "openai/" + # prefix, which reads OPENAI_API_KEY from the environment. The + # harness runs in-process, so set the var for the duration of the + # run and restore the previous value afterwards. + prev_key = os.environ.get("OPENAI_API_KEY") + os.environ["OPENAI_API_KEY"] = api_key + try: + results = backend.run_harness(run_id) + finally: + if prev_key is None: + os.environ.pop("OPENAI_API_KEY", None) + else: + os.environ["OPENAI_API_KEY"] = prev_key + else: + results = backend.run_harness(run_id) # Convert BenchmarkResults to RunSummary total = len(results.trial_results) if hasattr(results, "trial_results") else 0 @@ -711,18 +764,47 @@ def _run_terminalbench_native(config, console: Console) -> object: ) -def _run_single(config, console: Optional[Console] = None) -> object: - """Run a single eval from a RunConfig and return the summary.""" +def _run_single( + config, + console: Optional[Console] = None, + *, + suite_mode: bool = False, +) -> object: + """Run a single eval from a RunConfig and return the summary. + + ``suite_mode=True`` (TOML-suite drivers) scopes ``config.base_url`` / + ``config.api_key`` — stamped from the suite's ``[backend.external]`` + section onto every RunConfig — to the hermes/openclaw backends only; + extending suite-level endpoint targeting to first-party backends is + explicitly deferred. The CLI single-run path (``suite_mode=False``) + honors ``--base-url``/``--api-key`` for every backend. + """ from openjarvis.evals.core.runner import EvalRunner if console is None: console = Console() + _metadata = getattr(config, "metadata", None) or {} + base_url = ( + getattr(config, "base_url", None) + or _metadata.get("base_url") + or os.environ.get("JARVIS_BACKEND_BASE_URL") + ) + api_key = ( + getattr(config, "api_key", None) + or _metadata.get("api_key") + or os.environ.get("JARVIS_BACKEND_API_KEY") + ) + # TerminalBench V2.1 native: use terminal-bench Harness directly if config.benchmark == "terminalbench-native": - return _run_terminalbench_native(config, console) + return _run_terminalbench_native( + config, + console, + base_url=None if suite_mode else base_url, + api_key=None if suite_mode else api_key, + ) - _metadata = getattr(config, "metadata", None) or {} eval_backend = _build_backend( config.backend, config.engine_key, @@ -732,16 +814,9 @@ def _run_single(config, console: Optional[Console] = None) -> object: gpu_metrics=getattr(config, "gpu_metrics", False), model=config.model, max_turns=getattr(config, "max_turns", None), - base_url=( - getattr(config, "base_url", None) - or _metadata.get("base_url") - or os.environ.get("JARVIS_BACKEND_BASE_URL") - ), - api_key=( - getattr(config, "api_key", None) - or _metadata.get("api_key") - or os.environ.get("JARVIS_BACKEND_API_KEY") - ), + base_url=base_url, + api_key=api_key, + first_party_endpoint=not suite_mode, ) dataset = _build_dataset(config.benchmark) # Inject engine config for benchmarks that run their own simulation @@ -1070,7 +1145,7 @@ def _run_from_config( f"Run {i}/{len(run_configs)}: {rc.benchmark} / {rc.model}", ) try: - summary = _run_single(rc, console=console) + summary = _run_single(rc, console=console, suite_mode=True) summaries.append(summary) console.print( f" [green]{summary.accuracy:.4f}[/green] " @@ -1115,12 +1190,21 @@ def main(): @click.option( "--base-url", default=None, - help="OpenAI-compat endpoint for hermes/openclaw", + help=( + "OpenAI-compatible endpoint for the model under eval. Required for " + "hermes/openclaw; for jarvis-direct/jarvis-agent/terminalbench-native " + "it bypasses engine discovery and targets this URL directly " + "(env: JARVIS_BACKEND_BASE_URL)." + ), ) @click.option( "--api-key", default=None, - help="API key for hermes/openclaw endpoint", + help=( + "API key for the --base-url endpoint, sent as a Bearer token. " + "Required for hermes/openclaw; optional for first-party backends " + "(env: JARVIS_BACKEND_API_KEY)." + ), ) @click.option("-m", "--model", default=None, help="Model identifier") @click.option( @@ -1526,8 +1610,7 @@ def summarize(jsonl_path): default=None, type=click.Path(), help=( - "Output JSONL path. Defaults to .reparsed when " - "--in-place is not set." + "Output JSONL path. Defaults to .reparsed when --in-place is not set." ), ) @click.option( @@ -1642,9 +1725,7 @@ def reparse_judge(jsonl_path, out_path, in_place, summary_out): _json.dump(summary, f, indent=2) old_cont = [float(s) for s in old_scores if s is not None] - old_acc = ( - sum(1 for s in old_cont if s >= 0.5) / len(old_cont) if old_cont else 0.0 - ) + old_acc = sum(1 for s in old_cont if s >= 0.5) / len(old_cont) if old_cont else 0.0 old_mean = sum(old_cont) / len(old_cont) if old_cont else 0.0 new_mean = sum(cont) / len(cont) if cont else 0.0 mean_shift = new_mean - old_mean diff --git a/src/openjarvis/evals/tests/test_backends.py b/src/openjarvis/evals/tests/test_backends.py index 7854d2fb..8c7fd25b 100644 --- a/src/openjarvis/evals/tests/test_backends.py +++ b/src/openjarvis/evals/tests/test_backends.py @@ -4,6 +4,27 @@ from __future__ import annotations from unittest.mock import MagicMock, patch +import httpx +import pytest +import respx + + +def _mock_builder() -> MagicMock: + """A SystemBuilder mock whose fluent methods chain like the real one.""" + builder = MagicMock() + for method in ( + "engine", + "engine_instance", + "model", + "agent", + "tools", + "telemetry", + "traces", + ): + getattr(builder, method).return_value = builder + builder.build.return_value = MagicMock() + return builder + class TestJarvisDirectBackend: @patch("openjarvis.system.SystemBuilder") @@ -137,3 +158,130 @@ class TestJarvisAgentBackend: assert result["content"] == "The answer is 4." assert result["turns"] == 2 assert len(result["tool_results"]) == 1 + + +class TestJarvisDirectBackendBaseUrl: + """--base-url targeting for the jarvis-direct backend.""" + + @patch("openjarvis.system.SystemBuilder") + def test_base_url_injects_pinned_openai_compat_engine(self, mock_builder_cls): + from openjarvis.engine.openai_compat_engines import OpenAICompatEngine + from openjarvis.evals.backends.jarvis_direct import JarvisDirectBackend + + mock_builder = _mock_builder() + mock_builder_cls.return_value = mock_builder + + with respx.mock: + respx.get("http://127.0.0.1:18999/v1/models").mock( + return_value=httpx.Response(200, json={"data": []}) + ) + JarvisDirectBackend(base_url="http://127.0.0.1:18999/v1", api_key="sk-x") + + mock_builder.engine_instance.assert_called_once() + injected = mock_builder.engine_instance.call_args[0][0] + assert isinstance(injected, OpenAICompatEngine) + # Trailing /v1 is normalized away so request paths don't double up. + assert injected._host == "http://127.0.0.1:18999" + assert injected._api_key == "sk-x" + # The discovery path must not be engaged at all. + mock_builder.engine.assert_not_called() + + @patch("openjarvis.system.SystemBuilder") + def test_unreachable_base_url_fails_fast_naming_url(self, mock_builder_cls): + from openjarvis.evals.backends.jarvis_direct import JarvisDirectBackend + + mock_builder = _mock_builder() + mock_builder_cls.return_value = mock_builder + + with respx.mock: + respx.get("http://127.0.0.1:18998/v1/models").mock( + side_effect=httpx.ConnectError("connection refused") + ) + with pytest.raises(RuntimeError, match=r"http://127\.0\.0\.1:18998"): + JarvisDirectBackend(base_url="http://127.0.0.1:18998") + + # No silent engine substitution: the system is never built. + mock_builder.engine_instance.assert_not_called() + mock_builder.build.assert_not_called() + + @patch("openjarvis.system.SystemBuilder") + def test_no_base_url_keeps_engine_key_path(self, mock_builder_cls): + from openjarvis.evals.backends.jarvis_direct import JarvisDirectBackend + + mock_builder = _mock_builder() + mock_builder_cls.return_value = mock_builder + + JarvisDirectBackend(engine_key="vllm") + mock_builder.engine.assert_called_with("vllm") + mock_builder.engine_instance.assert_not_called() + + @patch("openjarvis.system.SystemBuilder") + def test_base_url_wins_over_engine_key(self, mock_builder_cls): + from openjarvis.evals.backends.jarvis_direct import JarvisDirectBackend + + mock_builder = _mock_builder() + mock_builder_cls.return_value = mock_builder + + with respx.mock: + respx.get("http://127.0.0.1:18999/v1/models").mock( + return_value=httpx.Response(200, json={"data": []}) + ) + JarvisDirectBackend(engine_key="vllm", base_url="http://127.0.0.1:18999") + + mock_builder.engine.assert_not_called() + mock_builder.engine_instance.assert_called_once() + # The engine key is kept as the label for the injected engine. + assert mock_builder.engine_instance.call_args.kwargs["key"] == "vllm" + + +class TestJarvisAgentBackendBaseUrl: + """--base-url targeting for the jarvis-agent backend.""" + + @patch("openjarvis.system.SystemBuilder") + def test_base_url_injects_pinned_openai_compat_engine(self, mock_builder_cls): + from openjarvis.engine.openai_compat_engines import OpenAICompatEngine + from openjarvis.evals.backends.jarvis_agent import JarvisAgentBackend + + mock_builder = _mock_builder() + mock_builder_cls.return_value = mock_builder + + with respx.mock: + respx.get("http://127.0.0.1:18999/v1/models").mock( + return_value=httpx.Response(200, json={"data": []}) + ) + JarvisAgentBackend(base_url="http://127.0.0.1:18999/v1", api_key="sk-x") + + mock_builder.engine_instance.assert_called_once() + injected = mock_builder.engine_instance.call_args[0][0] + assert isinstance(injected, OpenAICompatEngine) + assert injected._host == "http://127.0.0.1:18999" + assert injected._api_key == "sk-x" + mock_builder.engine.assert_not_called() + + @patch("openjarvis.system.SystemBuilder") + def test_unreachable_base_url_fails_fast_naming_url(self, mock_builder_cls): + from openjarvis.evals.backends.jarvis_agent import JarvisAgentBackend + + mock_builder = _mock_builder() + mock_builder_cls.return_value = mock_builder + + with respx.mock: + respx.get("http://127.0.0.1:18998/v1/models").mock( + side_effect=httpx.ConnectError("connection refused") + ) + with pytest.raises(RuntimeError, match=r"http://127\.0\.0\.1:18998"): + JarvisAgentBackend(base_url="http://127.0.0.1:18998") + + mock_builder.engine_instance.assert_not_called() + mock_builder.build.assert_not_called() + + @patch("openjarvis.system.SystemBuilder") + def test_no_base_url_keeps_engine_key_path(self, mock_builder_cls): + from openjarvis.evals.backends.jarvis_agent import JarvisAgentBackend + + mock_builder = _mock_builder() + mock_builder_cls.return_value = mock_builder + + JarvisAgentBackend(engine_key="vllm") + mock_builder.engine.assert_called_with("vllm") + mock_builder.engine_instance.assert_not_called() diff --git a/src/openjarvis/evals/tests/test_cli_endpoint.py b/src/openjarvis/evals/tests/test_cli_endpoint.py new file mode 100644 index 00000000..8257e61f --- /dev/null +++ b/src/openjarvis/evals/tests/test_cli_endpoint.py @@ -0,0 +1,190 @@ +"""--base-url/--api-key forwarding through the eval CLI plumbing. + +Covers the fix for the eval-CLI endpoint gap: the flags used to be silently +dropped for jarvis-direct/jarvis-agent and ignored by terminalbench-native +(which hardcoded api_base="http://localhost:8000/v1"). +""" + +from __future__ import annotations + +import io +import os +from types import SimpleNamespace +from unittest.mock import MagicMock, patch + +import click +import pytest +from rich.console import Console + +from openjarvis.evals.cli import _build_backend, _run_terminalbench_native +from openjarvis.evals.core.types import RunConfig + + +def _quiet_console() -> Console: + return Console(file=io.StringIO()) + + +def _tb_config(**overrides) -> RunConfig: + defaults = dict( + benchmark="terminalbench-native", + backend="jarvis-direct", + model="my-model", + max_samples=1, + max_workers=1, + temperature=0.2, + ) + defaults.update(overrides) + return RunConfig(**defaults) + + +class TestBuildBackendForwardsEndpoint: + @patch("openjarvis.evals.backends.jarvis_direct.JarvisDirectBackend") + def test_jarvis_direct_receives_base_url_and_api_key(self, mock_cls): + _build_backend( + "jarvis-direct", + "vllm", + "orchestrator", + [], + base_url="http://node7:8123/v1", + api_key="sk-k", + ) + kwargs = mock_cls.call_args.kwargs + assert kwargs["base_url"] == "http://node7:8123/v1" + assert kwargs["api_key"] == "sk-k" + + @patch("openjarvis.evals.backends.jarvis_agent.JarvisAgentBackend") + def test_jarvis_agent_receives_base_url_and_api_key(self, mock_cls): + _build_backend( + "jarvis-agent", + "vllm", + "orchestrator", + ["calculator"], + base_url="http://node7:8123/v1", + api_key="sk-k", + ) + kwargs = mock_cls.call_args.kwargs + assert kwargs["base_url"] == "http://node7:8123/v1" + assert kwargs["api_key"] == "sk-k" + + @patch("openjarvis.evals.backends.jarvis_direct.JarvisDirectBackend") + def test_suite_mode_scopes_endpoint_to_external_backends(self, mock_cls): + """[backend.external] suite semantics stay hermes/openclaw-only: + first_party_endpoint=False must not forward to first-party.""" + _build_backend( + "jarvis-direct", + "vllm", + "orchestrator", + [], + base_url="http://node7:8123/v1", + api_key="sk-k", + first_party_endpoint=False, + ) + kwargs = mock_cls.call_args.kwargs + assert kwargs["base_url"] is None + assert kwargs["api_key"] is None + + def test_hermes_still_requires_base_url_and_api_key(self): + with pytest.raises(click.UsageError, match="hermes"): + _build_backend("hermes", None, "orchestrator", []) + + def test_openclaw_still_requires_base_url_and_api_key(self): + with pytest.raises(click.UsageError, match="openclaw"): + _build_backend("openclaw", None, "orchestrator", []) + + +class TestTerminalBenchNativeApiBase: + @patch("openjarvis.evals.backends.terminalbench_native.TerminalBenchNativeBackend") + def test_base_url_passed_through_as_api_base(self, mock_cls): + mock_backend = MagicMock() + mock_backend.run_harness.return_value = SimpleNamespace(trial_results=[]) + mock_cls.return_value = mock_backend + + _run_terminalbench_native( + _tb_config(), + _quiet_console(), + base_url="http://node7:8123/v1", + ) + assert mock_cls.call_args.kwargs["api_base"] == "http://node7:8123/v1" + + @patch("openjarvis.evals.backends.terminalbench_native.TerminalBenchNativeBackend") + def test_base_url_without_v1_gets_single_v1_suffix(self, mock_cls): + mock_backend = MagicMock() + mock_backend.run_harness.return_value = SimpleNamespace(trial_results=[]) + mock_cls.return_value = mock_backend + + _run_terminalbench_native( + _tb_config(), + _quiet_console(), + base_url="http://node7:8123", + ) + assert mock_cls.call_args.kwargs["api_base"] == "http://node7:8123/v1" + + @patch("openjarvis.evals.backends.terminalbench_native.TerminalBenchNativeBackend") + def test_default_api_base_unchanged_without_base_url(self, mock_cls): + mock_backend = MagicMock() + mock_backend.run_harness.return_value = SimpleNamespace(trial_results=[]) + mock_cls.return_value = mock_backend + + _run_terminalbench_native(_tb_config(), _quiet_console()) + assert mock_cls.call_args.kwargs["api_base"] == "http://localhost:8000/v1" + + @patch("openjarvis.evals.backends.terminalbench_native.TerminalBenchNativeBackend") + def test_api_key_exported_as_openai_api_key_during_run(self, mock_cls, monkeypatch): + """terminus-2 reads OPENAI_API_KEY via LiteLLM; the var must be set + during harness.run() and restored afterwards.""" + monkeypatch.delenv("OPENAI_API_KEY", raising=False) + seen: dict = {} + + def fake_run_harness(run_id): + seen["openai_api_key"] = os.environ.get("OPENAI_API_KEY") + return SimpleNamespace(trial_results=[]) + + mock_backend = MagicMock() + mock_backend.run_harness.side_effect = fake_run_harness + mock_cls.return_value = mock_backend + + _run_terminalbench_native( + _tb_config(), + _quiet_console(), + base_url="http://node7:8123/v1", + api_key="sk-tb", + ) + assert seen["openai_api_key"] == "sk-tb" + assert "OPENAI_API_KEY" not in os.environ # restored + + @patch("openjarvis.evals.backends.terminalbench_native.TerminalBenchNativeBackend") + def test_preexisting_openai_api_key_restored(self, mock_cls, monkeypatch): + monkeypatch.setenv("OPENAI_API_KEY", "sk-original") + mock_backend = MagicMock() + mock_backend.run_harness.return_value = SimpleNamespace(trial_results=[]) + mock_cls.return_value = mock_backend + + _run_terminalbench_native( + _tb_config(), + _quiet_console(), + base_url="http://node7:8123/v1", + api_key="sk-tb", + ) + assert os.environ["OPENAI_API_KEY"] == "sk-original" + + +class TestRunSingleSuiteModeGating: + @patch("openjarvis.evals.cli._run_terminalbench_native") + def test_suite_mode_drops_endpoint_for_terminalbench(self, mock_tb): + from openjarvis.evals.cli import _run_single + + mock_tb.return_value = SimpleNamespace(accuracy=0.0) + config = _tb_config(base_url="http://node7:8123/v1", api_key="sk-k") + _run_single(config, console=_quiet_console(), suite_mode=True) + assert mock_tb.call_args.kwargs["base_url"] is None + assert mock_tb.call_args.kwargs["api_key"] is None + + @patch("openjarvis.evals.cli._run_terminalbench_native") + def test_cli_mode_forwards_endpoint_for_terminalbench(self, mock_tb): + from openjarvis.evals.cli import _run_single + + mock_tb.return_value = SimpleNamespace(accuracy=0.0) + config = _tb_config(base_url="http://node7:8123/v1", api_key="sk-k") + _run_single(config, console=_quiet_console()) + assert mock_tb.call_args.kwargs["base_url"] == "http://node7:8123/v1" + assert mock_tb.call_args.kwargs["api_key"] == "sk-k" diff --git a/src/openjarvis/system/builder.py b/src/openjarvis/system/builder.py index a5a317c3..c686c504 100644 --- a/src/openjarvis/system/builder.py +++ b/src/openjarvis/system/builder.py @@ -33,6 +33,8 @@ class SystemBuilder: self._config = load_config() self._engine_key: Optional[str] = None + self._engine_instance: Optional[InferenceEngine] = None + self._engine_instance_key: Optional[str] = None self._model: Optional[str] = None self._agent_name: Optional[str] = None self._tool_names: Optional[List[str]] = None @@ -50,6 +52,20 @@ class SystemBuilder: self._engine_key = key return self + def engine_instance( + self, engine: InferenceEngine, key: str = "openai-compat" + ) -> SystemBuilder: + """Inject a pre-built engine instance, bypassing engine discovery. + + Used by callers that must target one exact endpoint (e.g. + ``jarvis eval --base-url``). ``build()`` health-checks the instance + and raises a loud error if it is unreachable — it never silently + substitutes a different discovered engine. + """ + self._engine_instance = engine + self._engine_instance_key = key + return self + def model(self, name: str) -> SystemBuilder: self._model = name return self @@ -303,6 +319,23 @@ class SystemBuilder: return system def _resolve_engine(self, config: JarvisConfig): + # An explicitly injected engine instance always wins and is never + # silently replaced: when the caller pinned an endpoint (e.g. + # ``jarvis eval --base-url``) and it is down, substituting whatever + # other engine discovery finds would silently run against the wrong + # model server. Fail loudly instead. + if self._engine_instance is not None: + engine = self._engine_instance + key = self._engine_instance_key or "openai-compat" + if not engine.health(): + host = getattr(engine, "_host", "") + raise RuntimeError( + f"Injected engine {key!r} is not reachable at {host} — " + "is the endpoint running and serving GET /v1/models? " + "Refusing to fall back to engine discovery." + ) + return engine, key + from openjarvis.engine._discovery import get_engine pref = config.intelligence.preferred_engine @@ -313,7 +346,18 @@ class SystemBuilder: "No inference engine available. " "Make sure an engine is running (e.g. ollama serve)." ) - return resolved[1], resolved[0] + resolved_key, engine = resolved + if self._engine_key and resolved_key != self._engine_key: + # get_engine() falls back to any healthy discovered engine; make + # the substitution visible when the caller asked for a specific + # engine (observed: requested vllm, silently got ollama@11434). + logger.warning( + "Requested engine %r is unavailable; using %r at %s instead", + self._engine_key, + resolved_key, + getattr(engine, "_host", ""), + ) + return engine, resolved_key def _resolve_model(self, config: JarvisConfig, engine: InferenceEngine) -> str: if self._model: diff --git a/tests/engine/test_openai_compat_api_key.py b/tests/engine/test_openai_compat_api_key.py new file mode 100644 index 00000000..dc541358 --- /dev/null +++ b/tests/engine/test_openai_compat_api_key.py @@ -0,0 +1,109 @@ +"""API-key (Authorization header) support in the OpenAI-compat engine base.""" + +from __future__ import annotations + +import httpx +import pytest +import respx + +from openjarvis.core.types import Message, Role +from openjarvis.engine.openai_compat_engines import ( + OpenAICompatEngine, + VLLMEngine, + normalize_openai_base_url, +) + +_CHAT_RESPONSE = { + "choices": [{"message": {"content": "ok"}, "finish_reason": "stop"}], + "usage": {"prompt_tokens": 1, "completion_tokens": 1, "total_tokens": 2}, + "model": "m", +} + + +class TestAuthorizationHeader: + def test_bearer_header_sent_when_api_key_set(self) -> None: + engine = OpenAICompatEngine(host="http://testhost:9000", api_key="sk-test") + with respx.mock: + route = respx.post("http://testhost:9000/v1/chat/completions").mock( + return_value=httpx.Response(200, json=_CHAT_RESPONSE) + ) + engine.generate([Message(role=Role.USER, content="hi")], model="m") + assert route.calls.last.request.headers["Authorization"] == "Bearer sk-test" + + def test_no_authorization_header_without_api_key(self) -> None: + engine = OpenAICompatEngine(host="http://testhost:9000") + with respx.mock: + route = respx.post("http://testhost:9000/v1/chat/completions").mock( + return_value=httpx.Response(200, json=_CHAT_RESPONSE) + ) + engine.generate([Message(role=Role.USER, content="hi")], model="m") + assert "authorization" not in route.calls.last.request.headers + + def test_health_check_sends_bearer_header(self) -> None: + engine = OpenAICompatEngine(host="http://testhost:9000", api_key="sk-test") + with respx.mock: + route = respx.get("http://testhost:9000/v1/models").mock( + return_value=httpx.Response(200, json={"data": []}) + ) + assert engine.health() is True + assert route.calls.last.request.headers["Authorization"] == "Bearer sk-test" + + def test_env_var_fallback_sanitizes_hyphen( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + # engine_id "openai-compat" must map to OPENAI_COMPAT_API_KEY — + # shells cannot set hyphenated env-var names. + monkeypatch.setenv("OPENAI_COMPAT_API_KEY", "sk-env") + engine = OpenAICompatEngine(host="http://testhost:9000") + with respx.mock: + route = respx.get("http://testhost:9000/v1/models").mock( + return_value=httpx.Response(200, json={"data": []}) + ) + engine.health() + assert route.calls.last.request.headers["Authorization"] == "Bearer sk-env" + + def test_vllm_env_var_fallback(self, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("VLLM_API_KEY", "sk-vllm") + engine = VLLMEngine(host="http://testhost:8000") + with respx.mock: + route = respx.get("http://testhost:8000/v1/models").mock( + return_value=httpx.Response(200, json={"data": []}) + ) + engine.health() + assert route.calls.last.request.headers["Authorization"] == "Bearer sk-vllm" + + def test_explicit_api_key_beats_env_var( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + monkeypatch.setenv("OPENAI_COMPAT_API_KEY", "sk-env") + engine = OpenAICompatEngine(host="http://testhost:9000", api_key="sk-explicit") + assert engine._api_key == "sk-explicit" + + +class TestNormalizeOpenAIBaseUrl: + @pytest.mark.parametrize( + ("url", "expected"), + [ + ("http://h:8000", "http://h:8000"), + ("http://h:8000/", "http://h:8000"), + ("http://h:8000/v1", "http://h:8000"), + ("http://h:8000/v1/", "http://h:8000"), + ("http://h:8000/gateway/v1", "http://h:8000/gateway"), + # Only a literal trailing "/v1" is stripped — never other paths. + ("http://h:8000/v1x", "http://h:8000/v1x"), + ("http://h:8000/v2", "http://h:8000/v2"), + ], + ) + def test_normalization(self, url: str, expected: str) -> None: + assert normalize_openai_base_url(url) == expected + + def test_engine_requests_have_single_v1_prefix(self) -> None: + """End to end: a user-supplied .../v1 URL must not produce /v1/v1.""" + host = normalize_openai_base_url("http://testhost:9000/v1") + engine = OpenAICompatEngine(host=host) + with respx.mock: + route = respx.get("http://testhost:9000/v1/models").mock( + return_value=httpx.Response(200, json={"data": []}) + ) + assert engine.health() is True + assert route.calls.last.request.url.path == "/v1/models" diff --git a/tests/sdk/test_system.py b/tests/sdk/test_system.py index bb1c9b0c..4502ea47 100644 --- a/tests/sdk/test_system.py +++ b/tests/sdk/test_system.py @@ -2,7 +2,7 @@ from __future__ import annotations -from unittest.mock import MagicMock +from unittest.mock import MagicMock, patch import pytest @@ -327,6 +327,85 @@ class TestSystemBuilder: assert builder._engine_key == "ollama" +class TestSystemBuilderEngineInstance: + """Explicit engine injection (jarvis eval --base-url path).""" + + @staticmethod + def _fake_engine(healthy: bool = True) -> MagicMock: + engine = MagicMock( + spec=["health", "can_serve", "generate", "list_models", "close"] + ) + engine.health.return_value = healthy + engine._host = "http://127.0.0.1:18999" + return engine + + def test_engine_instance_is_fluent(self): + builder = SystemBuilder(JarvisConfig()) + engine = self._fake_engine() + result = builder.engine_instance(engine, key="my-endpoint") + assert result is builder + assert builder._engine_instance is engine + assert builder._engine_instance_key == "my-endpoint" + + def test_resolve_engine_returns_injected_instance(self): + config = JarvisConfig() + engine = self._fake_engine(healthy=True) + builder = SystemBuilder(config).engine_instance(engine, key="endpoint") + resolved_engine, resolved_key = builder._resolve_engine(config) + assert resolved_engine is engine + assert resolved_key == "endpoint" + + def test_unhealthy_injected_instance_raises_naming_host(self): + config = JarvisConfig() + engine = self._fake_engine(healthy=False) + builder = SystemBuilder(config).engine_instance(engine, key="endpoint") + with pytest.raises(RuntimeError, match=r"http://127\.0\.0\.1:18999"): + builder._resolve_engine(config) + + def test_unhealthy_injected_instance_never_consults_discovery(self): + """The observed failure mode: an explicit endpoint must NOT be + silently replaced by whatever other engine discovery finds.""" + config = JarvisConfig() + engine = self._fake_engine(healthy=False) + builder = SystemBuilder(config).engine_instance(engine) + with patch("openjarvis.engine._discovery.get_engine") as mock_get_engine: + with pytest.raises(RuntimeError, match="Refusing to fall back"): + builder._resolve_engine(config) + mock_get_engine.assert_not_called() + + def test_healthy_injected_instance_never_consults_discovery(self): + config = JarvisConfig() + engine = self._fake_engine(healthy=True) + builder = SystemBuilder(config).engine_instance(engine, key="endpoint") + with patch("openjarvis.engine._discovery.get_engine") as mock_get_engine: + resolved_engine, _ = builder._resolve_engine(config) + assert resolved_engine is engine + mock_get_engine.assert_not_called() + + def test_build_wires_injected_engine(self): + """build() must use the injected engine (possibly behind security + wrappers) instead of running discovery.""" + config = JarvisConfig() + engine = self._fake_engine(healthy=True) + engine.list_models.return_value = ["stub-model"] + builder = ( + SystemBuilder(config) + .engine_instance(engine, key="endpoint") + .model("stub-model") + .telemetry(False) + .traces(False) + ) + system = builder.build() + try: + inner = system.engine + while hasattr(inner, "_engine"): + inner = inner._engine + assert inner is engine + assert system.engine_key == "endpoint" + finally: + system.close() + + class TestJarvisSystemClose: def test_close_with_scheduler_store(self): engine = MagicMock()