Compare commits

...
Author SHA1 Message Date
03c5ec3e40 fix(chat): wire SystemPromptBuilder so persona files load in jarvis chat (fixes #458) (#497)
* fix(chat): wire SystemPromptBuilder so persona files load in jarvis chat (fixes #458)

* fix(chat): make `--persona none` actually disable persona files

This PR exposes `--persona none`, but SystemPromptBuilder._load_file read
empty paths as "." (Path("") -> ".") and raised IsADirectoryError, so the
documented opt-out crashed. Guard empty path_str so the "none" opt-out
(which _resolve_persona maps to empty file paths) cleanly injects no
persona. Adds an end-to-end regression test (building with persona
"none" must not raise). Also merges current main (branch was stale).

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>
2026-06-10 14:10:13 -07:00
Jon Saad-FalconandClaude Opus 4.8 4218258486 perf(serve): build the system once — drop duplicate SystemBuilder.build() (#263) (#529)
`jarvis serve` constructed every heavy component inline (engine discovery +
instrumentation, telemetry, memory, agent manager, per-agent tools) and then
called `SystemBuilder(config).build()` a second time inside the scheduler
block purely to feed `AgentExecutor.set_system()`. That second build
re-discovered and re-connected the engine, re-instrumented it, re-resolved
tools, re-opened the configured channel and re-created the agent manager —
~30-40s of fully redundant startup work (the headline remaining cost in #263
after engine probes were parallelised and the version check moved off the hot
path in #470).

Fix: assemble the executor's `JarvisSystem` from the components already built
inline instead of rebuilding from scratch. `AgentExecutor` only reads
`engine`, `model`, `config`, `memory_backend`, `tool_executor`,
`session_store` and `channel_backend` off the system; all are wired here. The
memory backend is now constructed just before the scheduler block (it was
built later) so the executor's system can reference it, and the primary
agent's resolved tool list is reused to build the scheduler's `ToolExecutor`
(preserving the MCP-discovered-tool pool the executor reads via
`tool_executor._tools`). `skill_manager` / the learning orchestrator are only
consumed by the orchestrator's `system.ask()` path, which the executor never
invokes, so they are intentionally omitted.

Tests: new `tests/cli/test_serve_single_build.py` patches
`SystemBuilder.build` and asserts it is never called during `jarvis serve`
startup, and that the executor still receives a system exposing
`tool_executor` / `session_store` / `memory_backend` (plus engine/model/config).

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-10 14:10:10 -07:00
Jon Saad-FalconandClaude Opus 4.8 176ed3029b fix(channels): unify send() destination/reply contract (Discord #515/#516) (#528)
The channel `send()` contract was inconsistent across adapters. Almost
every adapter (Discord, Slack, email, WhatsApp, ...) treats the first
positional `channel` arg as the real DESTINATION id and `conversation_id`
as an optional reply/thread reference. Telegram alone treated
`conversation_id` as the destination (`chat_id = conversation_id or
channel`).

`JarvisSystem._on_channel_message` hard-coded the Telegram-shaped mapping
for ALL channels: `send(cm.channel, reply, conversation_id=cm.conversation_id)`.
Since inbound `ChannelMessage`s carry the channel TYPE label in `.channel`
("discord") and the real destination id in `.conversation_id`, this sent
the literal "discord" as the Discord channel id (HTTP 400
NUMBER_TYPE_COERCE, #515) and passed the channel id as a Discord
`message_reference` (MESSAGE_REFERENCE_UNKNOWN_MESSAGE, #516).

Fix: define and document ONE canonical contract on `BaseChannel.send` —
positional `channel` = destination id, `conversation_id` = inbound message
id used as a reply reference — and dispatch it from `_on_channel_message`
as `send(cm.conversation_id, reply, conversation_id=cm.message_id)`,
matching the already-fixed `ChannelAgent` path (#495/#459). Telegram's
`send()` is brought into line (destination = `channel`, with a
`reply_to_message_id` reply ref and a legacy `conversation_id`-only
fallback) so it keeps working unchanged.

The DiscordChannel `_gateway_loop` ChannelMessage shape locked by #495 is
untouched; `tests/agents/test_channel_agent.py` passes unchanged. The
stale `test_serve_channel_wiring.py` assertions (which encoded the old
buggy mapping from #94) are updated, and regression tests are added for
Discord (real channel id + correct message_reference), Telegram (chat id
+ reply ref + legacy fallback), and per-channel dispatch in
`_on_channel_message`.

Fixes #515
Fixes #516

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-10 14:10:06 -07:00
11 changed files with 586 additions and 27 deletions
+21 -1
View File
@@ -60,7 +60,27 @@ class BaseChannel(ABC):
conversation_id: str = "",
metadata: Dict[str, Any] | None = None,
) -> bool:
"""Send a message to a specific channel. Returns True on success."""
"""Send a message to a specific channel. Returns True on success.
Canonical send contract shared by **every** channel adapter:
``channel``
The DESTINATION identifier — the per-adapter native id of the
place the message goes (Discord/Slack channel id, Telegram chat
id, email recipient address, ...). This is *not* the channel
TYPE label. An incoming :class:`ChannelMessage` carries that
destination in its ``conversation_id`` field (``channel`` there
is only the type label such as ``"discord"``), so dispatch code
replying to a message must pass ``cm.conversation_id`` here.
``conversation_id``
An optional reply/thread reference — the native id of the
message being replied to (Discord ``message_reference``, Slack
``thread_ts``, Telegram ``reply_to_message_id``, email
``In-Reply-To``, ...). When replying to an inbound message this
should be ``cm.message_id``, never the channel id. Passing a
channel id here yields broken references (e.g. Discord
``MESSAGE_REFERENCE_UNKNOWN_MESSAGE``).
"""
@abstractmethod
def status(self) -> ChannelStatus:
+11 -1
View File
@@ -112,7 +112,15 @@ class TelegramChannel(BaseChannel):
_TELEGRAM_MAX_LEN = 4096
url = f"https://api.telegram.org/bot{self._token}/sendMessage"
chat_id = conversation_id or channel
# Canonical channel send contract (see BaseChannel.send): the first
# positional ``channel`` arg is the DESTINATION (the Telegram chat
# id). ``conversation_id`` is the inbound message id used as a
# reply/thread reference (``reply_to_message_id``). We fall back to
# ``conversation_id`` as the chat id only when ``channel`` is empty,
# for backwards compatibility with legacy callers that passed the
# chat id via ``conversation_id``.
chat_id = channel or conversation_id
reply_to = conversation_id if (channel and conversation_id) else ""
chunks = textwrap.wrap(
content,
width=_TELEGRAM_MAX_LEN,
@@ -126,6 +134,8 @@ class TelegramChannel(BaseChannel):
}
if self._parse_mode:
payload["parse_mode"] = self._parse_mode
if reply_to:
payload["reply_to_message_id"] = reply_to
resp = httpx.post(url, json=payload, timeout=10.0)
if resp.status_code >= 300:
+43
View File
@@ -28,12 +28,22 @@ def _read_input(prompt: str = "You> ") -> Optional[str]:
@click.option("-a", "--agent", "agent_name", default=None, help="Agent type.")
@click.option("--tools", default=None, help="Comma-separated tool names.")
@click.option("--system", "system_prompt", default=None, help="Custom system prompt.")
@click.option(
"--persona",
"persona_name",
default=None,
help=(
"Named persona dir under ~/.openjarvis/personas/<name>/ "
"(overrides config). Pass 'none' to disable all persona files."
),
)
def chat(
engine_key: str | None,
model_name: str | None,
agent_name: str | None,
tools: str | None,
system_prompt: str | None,
persona_name: str | None,
) -> None:
"""Start an interactive multi-turn chat session.
@@ -48,6 +58,14 @@ def chat(
config = load_config()
import dataclasses as _dc
effective_mf = (
_dc.replace(config.memory_files, persona_name=persona_name)
if persona_name is not None
else config.memory_files
)
# Resolve engine
from openjarvis.engine import get_engine
from openjarvis.intelligence import register_builtin_models
@@ -121,6 +139,21 @@ def chat(
kwargs["interactive"] = True
kwargs["confirm_callback"] = _confirm
import inspect as _inspect
if (
"prompt_builder"
in _inspect.signature(agent_cls.__init__).parameters
):
from openjarvis.prompt.builder import SystemPromptBuilder
kwargs["prompt_builder"] = SystemPromptBuilder(
agent_template=config.agent.default_system_prompt or "",
memory_files_config=effective_mf,
system_prompt_config=config.system_prompt,
)
agent = agent_cls(engine, model, **kwargs)
except Exception as exc:
console.print(f"[yellow]Agent '{agent_key}' failed: {exc}[/yellow]")
@@ -147,6 +180,16 @@ def chat(
_notifications = NotificationDispatcher(get_status())
# Conversation state
if not system_prompt:
from openjarvis.prompt.builder import SystemPromptBuilder
builder = SystemPromptBuilder(
agent_template=config.agent.default_system_prompt or "",
memory_files_config=effective_mf,
system_prompt_config=config.system_prompt,
)
system_prompt = builder.build()
history: List[Message] = []
if system_prompt:
history.append(Message(role=Role.SYSTEM, content=system_prompt))
+73 -19
View File
@@ -261,6 +261,11 @@ def serve(
# Resolve agent
agent = None
agent_key = agent_name or config.server.agent
# Tool instances resolved for the primary agent are reused below to build
# the scheduler's ToolExecutor — avoiding a second full SystemBuilder.build()
# (which would re-discover the engine, re-resolve tools, re-open the channel,
# etc.). See the scheduler block near the bottom of this function (#263).
resolved_tools: list = []
if agent_key:
try:
import openjarvis.agents # noqa: F401
@@ -328,6 +333,8 @@ def serve(
if tools:
agent_kwargs["tools"] = tools
# Reuse these for the scheduler's ToolExecutor (#263).
resolved_tools = tools
if getattr(agent_cls, "accepts_tools", False):
agent_kwargs["max_turns"] = config.agent.max_turns
@@ -461,6 +468,24 @@ def serve(
# Create app
from openjarvis.server.app import create_app
# Set up memory backend for context injection. Built before the scheduler
# block so the executor's JarvisSystem can reference it (#263).
memory_backend = None
if config.agent.context_from_memory:
try:
import openjarvis.tools.storage # noqa: F401
from openjarvis.core.registry import MemoryRegistry
mem_key = config.memory.default_backend
if MemoryRegistry.contains(mem_key):
memory_backend = MemoryRegistry.create(
mem_key,
db_path=config.memory.db_path,
)
console.print(" Memory: [cyan]active[/cyan]")
except Exception as exc:
logger.debug("Memory backend init failed: %s", exc)
# Set up agent manager
agent_manager = None
if config.agent_manager.enabled:
@@ -500,9 +525,55 @@ def serve(
event_bus=bus,
trace_store=_trace_store,
)
from openjarvis.system import SystemBuilder
# Reuse the components already built inline above instead of a
# second full SystemBuilder.build() — the original double-build
# re-discovered the engine, re-instrumented it, re-resolved tools,
# re-opened the channel and re-created the agent manager, costing
# ~30-40s on top of an already-paid startup (#263). The executor
# only reads engine/model/config/memory_backend/tool_executor/
# session_store/channel_backend from the system (see
# AgentExecutor), all of which are wired here.
from openjarvis.sessions.session import SessionStore
from openjarvis.system import JarvisSystem
from openjarvis.tools._stubs import ToolExecutor
system = SystemBuilder(config).build()
_sched_session_store = None
if config.sessions.enabled:
try:
from pathlib import Path as _SchedPath
_sched_session_store = SessionStore(
db_path=_SchedPath(config.sessions.db_path).expanduser(),
max_age_hours=config.sessions.max_age_hours,
consolidation_threshold=(
config.sessions.consolidation_threshold
),
)
except Exception as exc:
logger.debug("Scheduler session store init failed: %s", exc)
_sched_tool_executor = (
ToolExecutor(resolved_tools, bus) if resolved_tools else None
)
system = JarvisSystem(
config=config,
bus=bus,
engine=engine,
engine_key=engine_name,
model=model_name,
agent=agent,
agent_name=agent_key or "",
tools=resolved_tools,
tool_executor=_sched_tool_executor,
memory_backend=memory_backend,
telemetry_store=telem_store,
trace_store=_trace_store,
session_store=_sched_session_store,
capability_policy=sec.capability_policy,
agent_manager=agent_manager,
agent_executor=executor,
)
executor.set_system(system)
agent_scheduler = AgentScheduler(
@@ -522,23 +593,6 @@ def serve(
except Exception as exc:
logger.debug("Agent scheduler init failed: %s", exc)
# Set up memory backend for context injection
memory_backend = None
if config.agent.context_from_memory:
try:
import openjarvis.tools.storage # noqa: F401
from openjarvis.core.registry import MemoryRegistry
mem_key = config.memory.default_backend
if MemoryRegistry.contains(mem_key):
memory_backend = MemoryRegistry.create(
mem_key,
db_path=config.memory.db_path,
)
console.print(" Memory: [cyan]active[/cyan]")
except Exception as exc:
logger.debug("Memory backend init failed: %s", exc)
# --- Channel Gateway: API key, sessions, ChannelBridge ---
import os as _os
+5
View File
@@ -229,6 +229,11 @@ class SystemPromptBuilder:
)
def _load_file(self, path_str: str, max_chars: int) -> str:
# An empty path means "no file" (e.g. the persona "none" opt-out, which
# resolves to empty paths). Guard before Path("") — which becomes "." —
# so reading it does not raise IsADirectoryError.
if not path_str:
return ""
path = Path(path_str).expanduser()
if not path.exists():
return ""
+12 -2
View File
@@ -268,10 +268,20 @@ class JarvisSystem:
if reply:
try:
# Canonical channel send contract (see BaseChannel.send):
# the first positional arg is the DESTINATION id, and the
# `conversation_id=` kwarg is the inbound message id used as
# a reply/thread reference. ``cm.conversation_id`` holds the
# real per-adapter destination (Discord/Slack channel id,
# Telegram chat id, ...) while ``cm.channel`` is only the
# channel TYPE label ("discord", "telegram", ...). Passing
# the type label as the destination produced HTTP 400s
# (#515) and using the channel id as a reply reference
# produced MESSAGE_REFERENCE_UNKNOWN_MESSAGE (#516).
channel_bridge.send(
cm.channel,
cm.conversation_id,
reply,
conversation_id=cm.conversation_id,
conversation_id=getattr(cm, "message_id", ""),
)
except Exception:
logger.exception("Channel send error")
+60
View File
@@ -130,3 +130,63 @@ class TestStatus:
ch = DiscordChannel()
ch.connect()
assert ch.status() == ChannelStatus.ERROR
class TestWireChannelEndToEnd:
"""Regression for #515/#516 — the full inbound→reply path through
JarvisSystem.wire_channel must call the real Discord REST API with the
numeric channel id (not "discord") and a message_reference equal to the
inbound message id (not the channel id).
"""
def test_reply_hits_real_channel_id_and_message_reference(self, tmp_path):
from openjarvis.channels._stubs import ChannelMessage
from openjarvis.core.config import JarvisConfig
from openjarvis.core.events import EventBus
from openjarvis.system import JarvisSystem
config = JarvisConfig()
config.sessions.db_path = str(tmp_path / "sessions.db")
from unittest.mock import MagicMock as _MM
system = JarvisSystem(
config=config,
bus=EventBus(record_history=False),
engine=_MM(),
engine_key="mock",
model="test-model",
agent_name="",
)
system.ask = _MM(return_value={"content": "pong"})
channel = DiscordChannel(bot_token="my-bot-token")
system.wire_channel(channel)
# Exactly the ChannelMessage shape DiscordChannel._gateway_loop emits:
# channel = "discord" (TYPE label), conversation_id = numeric channel
# id, message_id = numeric message id.
cm = ChannelMessage(
channel="discord",
sender="user-1",
content="hello",
message_id="111122223333444455",
conversation_id="987654321098765432",
)
mock_response = MagicMock()
mock_response.status_code = 200
with patch("httpx.post", return_value=mock_response) as mock_post:
# Invoke the handler wire_channel registered on the channel.
for handler in channel._handlers:
handler(cm)
mock_post.assert_called_once()
url = mock_post.call_args[0][0]
# #515: destination is the numeric channel id, not the "discord" label.
assert "discord.com/api/v10/channels/987654321098765432/messages" in url
assert "channels/discord/messages" not in url
payload = mock_post.call_args[1]["json"]
assert payload["content"] == "pong"
# #516: message_reference is the inbound message id, NOT the channel id.
assert payload["message_reference"] == {"message_id": "111122223333444455"}
assert payload["message_reference"]["message_id"] != "987654321098765432"
+35
View File
@@ -105,6 +105,41 @@ class TestSend:
event_types = [e.event_type for e in bus.history]
assert EventType.CHANNEL_MESSAGE_SENT in event_types
def test_send_uses_channel_as_chat_id_under_unified_contract(self):
"""Canonical contract (#515/#516): the first positional ``channel``
arg is the chat destination, and ``conversation_id`` is the inbound
message id used as ``reply_to_message_id`` — not the chat id."""
ch = TelegramChannel(bot_token="123:ABC")
mock_response = MagicMock()
mock_response.status_code = 200
with patch("httpx.post", return_value=mock_response) as mock_post:
result = ch.send("12345678", "Reply!", conversation_id="55")
assert result is True
payload = mock_post.call_args[1]["json"]
# Destination is the chat id from the positional channel arg.
assert payload["chat_id"] == "12345678"
# conversation_id becomes the reply reference, not the chat id.
assert payload["reply_to_message_id"] == "55"
def test_send_legacy_conversation_id_only_still_targets_chat(self):
"""Backwards compatibility: a legacy caller passing the chat id via
``conversation_id`` (with an empty ``channel``) still delivers."""
ch = TelegramChannel(bot_token="123:ABC")
mock_response = MagicMock()
mock_response.status_code = 200
with patch("httpx.post", return_value=mock_response) as mock_post:
result = ch.send("", "Hello!", conversation_id="12345678")
assert result is True
payload = mock_post.call_args[1]["json"]
assert payload["chat_id"] == "12345678"
# When channel is empty, conversation_id is the chat id, so it must
# not also be used as a self-referential reply id.
assert "reply_to_message_id" not in payload
class TestStatus:
def test_no_token_connect_error(self):
+96 -4
View File
@@ -79,10 +79,14 @@ class TestWireChannelWithAgent:
system.ask.assert_called_once()
assert system.ask.call_args[0][0] == "ping"
# Canonical send contract (#515/#516): the destination is the real
# per-adapter id (carried in ChannelMessage.conversation_id), not the
# channel TYPE label, and the conversation_id kwarg is the inbound
# message id used as a reply reference, not the channel id.
mock_channel.send.assert_called_once_with(
"telegram",
"42",
"pong",
conversation_id="42",
conversation_id="1",
)
def test_session_store_created_lazily(self, tmp_path):
@@ -126,13 +130,101 @@ class TestWireChannelWithEngine:
handler = mock_channel.on_message.call_args[0][0]
handler(_make_channel_message(content="hi"))
# Canonical send contract (#515/#516): destination = real channel id
# (from ChannelMessage.conversation_id), reply ref = inbound message id.
mock_channel.send.assert_called_once_with(
"telegram",
"42",
"raw reply",
conversation_id="42",
conversation_id="1",
)
class TestWireChannelCanonicalContract:
"""Regression for #515/#516 — wire_channel must dispatch the canonical
send contract so each adapter receives the right destination/reply ids,
regardless of the channel TYPE label.
"""
def test_discord_uses_real_channel_id_not_type_label(self, tmp_path):
"""A Discord ChannelMessage (channel="discord" TYPE label,
conversation_id=<numeric channel id>, message_id=<numeric msg id>)
must reply to the numeric channel id, with the message id as the
reply reference — never "discord" as destination (#515) and never the
channel id as the message reference (#516).
"""
system = _make_system(tmp_path=tmp_path)
system.ask = MagicMock(return_value={"content": "pong"})
mock_channel = MagicMock()
system.wire_channel(mock_channel)
handler = mock_channel.on_message.call_args[0][0]
cm = ChannelMessage(
channel="discord",
sender="user-1",
content="hello",
message_id="111122223333444455",
conversation_id="987654321098765432",
)
handler(cm)
args, kwargs = mock_channel.send.call_args
# Destination is the real Discord channel id, NOT the type label.
assert args[0] == "987654321098765432"
assert args[0] != "discord"
# Reply reference is the inbound message id, NOT the channel id.
assert kwargs["conversation_id"] == "111122223333444455"
assert kwargs["conversation_id"] != "987654321098765432"
def test_telegram_uses_chat_id_and_message_id(self, tmp_path):
"""Telegram must keep working under the unified contract: destination
is the chat id (conversation_id), reply ref is the message id."""
system = _make_system(tmp_path=tmp_path)
system.ask = MagicMock(return_value={"content": "pong"})
mock_channel = MagicMock()
system.wire_channel(mock_channel)
handler = mock_channel.on_message.call_args[0][0]
cm = ChannelMessage(
channel="telegram",
sender="42",
content="ping",
message_id="55",
conversation_id="12345678",
)
handler(cm)
mock_channel.send.assert_called_once_with(
"12345678",
"pong",
conversation_id="55",
)
def test_session_key_still_uses_conversation_id(self, tmp_path):
"""The fix must not change session isolation keying, which still uses
``<channel>:<conversation_id>``."""
system = _make_system(tmp_path=tmp_path)
system.ask = MagicMock(return_value={"content": "ok"})
mock_channel = MagicMock()
system.wire_channel(mock_channel)
handler = mock_channel.on_message.call_args[0][0]
cm = ChannelMessage(
channel="discord",
sender="u1",
content="hi",
message_id="msg-9",
conversation_id="chan-7",
)
handler(cm)
# Session was created under the channel:conversation_id key.
session = system.session_store.get_or_create("discord:chan-7")
assert any(m.content == "hi" for m in session.messages)
class TestWireChannelSessionIsolation:
"""Separate conversation_ids get independent sessions."""
+210
View File
@@ -0,0 +1,210 @@
"""Regression tests for #263 — ``jarvis serve`` must build the system once.
serve.py used to construct all heavy components inline and then call
``SystemBuilder(config).build()`` a second time inside the scheduler block,
re-discovering the engine, re-instrumenting it, re-resolving tools, re-opening
the channel and re-creating the agent manager — ~30-40s of redundant work.
These tests pin the fix:
1. ``SystemBuilder.build`` is never called during ``jarvis serve`` startup
(the duplicate build is gone).
2. The ``AgentExecutor`` still receives a system exposing the attributes it
actually reads: ``tool_executor``, ``session_store``, ``memory_backend``,
plus ``engine`` / ``model`` / ``config``.
"""
from __future__ import annotations
import importlib
from unittest.mock import MagicMock, patch
import pytest
from click.testing import CliRunner
from openjarvis.cli import cli
pytest.importorskip("fastapi")
pytest.importorskip("uvicorn")
# ``openjarvis.cli.serve`` as an attribute resolves to the click *command*
# (re-exported on the package); grab the real module to monkeypatch its globals.
serve_mod = importlib.import_module("openjarvis.cli.serve")
def _fake_engine() -> MagicMock:
engine = MagicMock()
engine.list_models.return_value = ["test-model"]
engine.health.return_value = True
engine.name = "mock"
return engine
def _repopulate_registries() -> None:
"""Re-run the @register decorators wiped by the autouse conftest fixture.
The tool/memory modules are import-cached, so a plain ``import`` inside
serve.py is a no-op after the registries are cleared per-test. Reload the
individual submodules so ToolRegistry/MemoryRegistry are populated exactly
as they would be on a fresh process — otherwise serve would resolve an
empty tool list and no memory backend, masking the very wiring under test.
"""
import importlib
import sys
import openjarvis.agents # noqa: F401
import openjarvis.tools # noqa: F401
import openjarvis.tools.storage # noqa: F401
from openjarvis.core.registry import (
AgentRegistry,
MemoryRegistry,
ToolRegistry,
)
if not AgentRegistry.keys():
for mod_name in list(sys.modules):
if mod_name.startswith("openjarvis.agents.") and not mod_name.endswith(
"_stubs"
):
try:
importlib.reload(sys.modules[mod_name])
except Exception:
pass
if not ToolRegistry.keys():
for mod_name in list(sys.modules):
if (
mod_name.startswith("openjarvis.tools.")
and not mod_name.endswith("_stubs")
and not mod_name.endswith("agent_tools")
):
try:
importlib.reload(sys.modules[mod_name])
except Exception:
pass
if not MemoryRegistry.keys():
for mod_name in list(sys.modules):
if mod_name.startswith(
"openjarvis.tools.storage."
) and not mod_name.endswith("_stubs"):
try:
importlib.reload(sys.modules[mod_name])
except Exception:
pass
def _run_serve(tmp_path, monkeypatch, *, build_spy, set_system_spy):
"""Invoke ``jarvis serve`` with all heavy/blocking pieces stubbed out.
Returns the CliRunner result. The server is never actually started
(``uvicorn.run`` is a no-op) and no real engine is contacted.
"""
from openjarvis.core.config import JarvisConfig
_repopulate_registries()
config = JarvisConfig()
# Keep the scheduler block alive (it owns the executor wiring under test)
# while pointing every store at the temp dir.
config.agent_manager.enabled = True
config.agent_manager.db_path = str(tmp_path / "agents.db")
config.sessions.enabled = True
config.sessions.db_path = str(tmp_path / "sessions.db")
config.memory.db_path = str(tmp_path / "memory.db")
config.telemetry.enabled = False
config.traces.enabled = False
config.channel.enabled = False
config.skills.enabled = False
config.server.host = "127.0.0.1"
config.server.port = 8123
# Resolve a model without contacting a real engine / discovery.
config.intelligence.default_model = "test-model"
engine = _fake_engine()
monkeypatch.setattr(serve_mod, "load_config", lambda *a, **k: config)
monkeypatch.setattr(serve_mod, "get_engine", lambda *a, **k: ("mock", engine))
monkeypatch.setattr(serve_mod, "discover_engines", lambda *a, **k: {})
monkeypatch.setattr(serve_mod, "discover_models", lambda *a, **k: {})
# setup_security returns its own context; pass the engine straight through
# so we don't need real guardrails wired up.
sec = MagicMock()
sec.engine = engine
sec.capability_policy = None
sec.audit_logger = None
monkeypatch.setattr("openjarvis.security.setup_security", lambda *a, **k: sec)
with (
patch(
"openjarvis.system.builder.SystemBuilder.build",
build_spy,
),
patch(
"openjarvis.agents.executor.AgentExecutor.set_system",
set_system_spy,
),
patch("uvicorn.run", lambda *a, **k: None),
):
return CliRunner().invoke(cli, ["serve"], catch_exceptions=False)
def test_serve_does_not_call_systembuilder_build(tmp_path, monkeypatch):
"""The redundant second full build is gone (#263)."""
build_spy = MagicMock(
side_effect=AssertionError(
"SystemBuilder.build() must not run during `jarvis serve` startup "
"— it is the duplicate build #263 removed."
)
)
set_system_spy = MagicMock()
result = _run_serve(
tmp_path,
monkeypatch,
build_spy=build_spy,
set_system_spy=set_system_spy,
)
assert result.exit_code == 0, result.output
build_spy.assert_not_called()
def test_executor_receives_required_system_attrs(tmp_path, monkeypatch):
"""The executor still gets a system exposing the attributes it reads.
AgentExecutor reads engine/model/config/memory_backend/tool_executor/
session_store off ``self._system``; the de-dup must not strip any of them.
"""
build_spy = MagicMock()
captured: dict = {}
def _capture_set_system(self, system): # noqa: ANN001
captured["system"] = system
# Preserve real behaviour so the executor is usable afterwards.
self._system = system
result = _run_serve(
tmp_path,
monkeypatch,
build_spy=build_spy,
set_system_spy=_capture_set_system,
)
assert result.exit_code == 0, result.output
# Built once, from the inline components — not via SystemBuilder.build().
build_spy.assert_not_called()
system = captured.get("system")
assert system is not None, "executor.set_system was never called"
# Correctness constraint from the verifier: these must survive the de-dup.
assert system.tool_executor is not None
assert system.session_store is not None
assert system.memory_backend is not None
# And the basics the executor resolves engine/model from.
assert system.engine is not None
assert system.model == "test-model"
assert system.config is not None
+20
View File
@@ -31,3 +31,23 @@ def test_named_persona_resolves_to_personas_dir():
def test_path_traversal_rejected(bad):
with pytest.raises(ValueError):
SystemPromptBuilder._resolve_persona(MemoryFilesConfig(persona_name=bad))
def test_none_persona_build_does_not_raise():
"""Regression (#497): `--persona none` resolves to empty file paths; building
the prompt must not raise IsADirectoryError when those empty paths are read
(Path("") is "." — reading a directory raised before the empty-path guard).
"""
import dataclasses
from openjarvis.core.config import load_config
cfg = load_config()
mf = dataclasses.replace(cfg.memory_files, persona_name="none")
builder = SystemPromptBuilder(
agent_template=cfg.agent.default_system_prompt or "",
memory_files_config=mf,
system_prompt_config=cfg.system_prompt,
)
out = builder.build()
assert isinstance(out, str)