mirror of
https://github.com/open-jarvis/OpenJarvis.git
synced 2026-08-15 01:12:06 +00:00
Compare commits
5
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
aec96f9d5d | ||
|
|
c2e1c375aa | ||
|
|
ef28e5f84b | ||
|
|
156d41d2f9 | ||
|
|
6e40d87eb5 |
@@ -127,6 +127,7 @@ class MonitorOperativeAgent(ToolUsingAgent):
|
||||
memory_backend: Optional[Any] = None,
|
||||
interactive: bool = False,
|
||||
confirm_callback=None,
|
||||
prompt_builder: Optional[Any] = None,
|
||||
**kwargs: Any,
|
||||
) -> None:
|
||||
super().__init__(
|
||||
@@ -139,7 +140,7 @@ class MonitorOperativeAgent(ToolUsingAgent):
|
||||
max_tokens=max_tokens,
|
||||
interactive=interactive,
|
||||
confirm_callback=confirm_callback,
|
||||
prompt_builder=kwargs.get("prompt_builder"),
|
||||
prompt_builder=prompt_builder,
|
||||
)
|
||||
# Validate strategies
|
||||
if memory_extraction not in VALID_MEMORY_EXTRACTION:
|
||||
|
||||
@@ -58,6 +58,7 @@ class OperativeAgent(ToolUsingAgent):
|
||||
memory_backend: Optional[Any] = None,
|
||||
interactive: bool = False,
|
||||
confirm_callback=None,
|
||||
prompt_builder: Optional[Any] = None,
|
||||
**kwargs: Any,
|
||||
) -> None:
|
||||
super().__init__(
|
||||
@@ -70,7 +71,7 @@ class OperativeAgent(ToolUsingAgent):
|
||||
max_tokens=max_tokens,
|
||||
interactive=interactive,
|
||||
confirm_callback=confirm_callback,
|
||||
prompt_builder=kwargs.get("prompt_builder"),
|
||||
prompt_builder=prompt_builder,
|
||||
)
|
||||
self._system_prompt = system_prompt or ""
|
||||
self._operator_id = operator_id
|
||||
|
||||
@@ -398,9 +398,8 @@ def _run_agent(
|
||||
|
||||
# Wire the SystemPromptBuilder so SOUL.md / MEMORY.md / USER.md persona
|
||||
# files actually reach the model. Only passed to agents whose __init__
|
||||
# accepts a `prompt_builder` kwarg (BaseAgent does; agents that override
|
||||
# __init__ without forwarding it, e.g. OrchestratorAgent, opt out
|
||||
# automatically and keep their existing system-prompt machinery).
|
||||
# explicitly accepts a `prompt_builder` kwarg. Agents with specialized
|
||||
# prompt machinery opt in by naming and forwarding the parameter.
|
||||
import inspect as _inspect
|
||||
|
||||
if "prompt_builder" in _inspect.signature(agent_cls.__init__).parameters:
|
||||
|
||||
@@ -311,12 +311,14 @@ def chat(
|
||||
# Generate response even when optional memory context is unavailable.
|
||||
try:
|
||||
if agent is not None:
|
||||
agent_context = None
|
||||
if agent_context_message is not None:
|
||||
from openjarvis.agents._stubs import AgentContext
|
||||
from openjarvis.agents._stubs import AgentContext
|
||||
|
||||
agent_context = AgentContext()
|
||||
agent_context = AgentContext()
|
||||
if agent_context_message is not None:
|
||||
agent_context.conversation.add(agent_context_message)
|
||||
for msg in history[:-1]:
|
||||
if msg.role != Role.SYSTEM:
|
||||
agent_context.conversation.add(msg)
|
||||
response = agent.run(user_input, context=agent_context)
|
||||
content = (
|
||||
response.content if hasattr(response, "content") else str(response)
|
||||
|
||||
@@ -367,6 +367,27 @@ def serve(
|
||||
if getattr(agent_cls, "accepts_tools", False):
|
||||
agent_kwargs["max_turns"] = config.agent.max_turns
|
||||
|
||||
# Wire the SystemPromptBuilder so SOUL.md / MEMORY.md / USER.md
|
||||
# reach the model on the SERVE path too. ``ask.py`` has done
|
||||
# this since the persona system landed; ``serve.py`` never did,
|
||||
# so an agent served over HTTP silently answered as a generic
|
||||
# assistant while the same agent via the CLI kept its persona.
|
||||
# Guarded so agents with specialized prompt machinery must opt
|
||||
# in by explicitly naming and forwarding the kwarg.
|
||||
import inspect as _inspect
|
||||
|
||||
if (
|
||||
"prompt_builder"
|
||||
in _inspect.signature(agent_cls.__init__).parameters
|
||||
):
|
||||
from openjarvis.prompt.builder import SystemPromptBuilder
|
||||
|
||||
agent_kwargs["prompt_builder"] = SystemPromptBuilder(
|
||||
agent_template=config.agent.default_system_prompt or "",
|
||||
memory_files_config=config.memory_files,
|
||||
system_prompt_config=config.system_prompt,
|
||||
)
|
||||
|
||||
agent = agent_cls(engine, model_name, **agent_kwargs)
|
||||
# Pin MCP transports to the agent's lifetime so HTTP
|
||||
# connections don't close mid-request (#461).
|
||||
|
||||
@@ -516,6 +516,20 @@ class Jarvis:
|
||||
existing = agent_kwargs.get("tools", [])
|
||||
agent_kwargs["tools"] = digest_tools + list(existing)
|
||||
|
||||
# Wire the SystemPromptBuilder so SOUL.md / MEMORY.md / USER.md reach
|
||||
# the model — mirrors ``cli/ask.py`` and ``cli/serve.py``. Guarded so
|
||||
# agents whose ``__init__`` doesn't accept the kwarg opt out.
|
||||
import inspect as _inspect
|
||||
|
||||
if "prompt_builder" in _inspect.signature(agent_cls.__init__).parameters:
|
||||
from openjarvis.prompt.builder import SystemPromptBuilder
|
||||
|
||||
agent_kwargs["prompt_builder"] = SystemPromptBuilder(
|
||||
agent_template=self._config.agent.default_system_prompt or "",
|
||||
memory_files_config=self._config.memory_files,
|
||||
system_prompt_config=self._config.system_prompt,
|
||||
)
|
||||
|
||||
agent_obj = agent_cls(self._engine, model_name, **agent_kwargs)
|
||||
ctx = AgentContext()
|
||||
|
||||
|
||||
@@ -396,11 +396,10 @@ class TestPersonaFilesReachModel:
|
||||
assert "MEMORY_SENTINEL" in joined
|
||||
assert "USER_SENTINEL" in joined
|
||||
|
||||
def test_orchestrator_keeps_its_own_system_prompt(
|
||||
def test_orchestrator_accepts_persona_prompt_builder(
|
||||
self, runner, monkeypatch, tmp_path
|
||||
):
|
||||
"""OrchestratorAgent's __init__ doesn't accept ``prompt_builder``;
|
||||
the wiring must skip it silently rather than crash."""
|
||||
"""Orchestrator explicitly accepts and applies persona wiring."""
|
||||
from openjarvis.core.config import JarvisConfig
|
||||
|
||||
soul = tmp_path / "SOUL.md"
|
||||
@@ -425,5 +424,6 @@ class TestPersonaFilesReachModel:
|
||||
):
|
||||
result = runner.invoke(cli, ["ask", "--agent", "orchestrator", "Hello"])
|
||||
|
||||
# Pass condition: doesn't crash with TypeError on prompt_builder kwarg.
|
||||
assert result.exit_code == 0, result.output
|
||||
messages = engine.generate.call_args.args[0]
|
||||
assert "ORCH_PERSONA_SENTINEL" in messages[0].content
|
||||
|
||||
+101
-1
@@ -17,7 +17,7 @@ from openjarvis.cli.chat_cmd import _read_input, chat
|
||||
from openjarvis.core.config import JarvisConfig
|
||||
from openjarvis.core.events import Event, EventBus, EventType
|
||||
from openjarvis.core.registry import AgentRegistry, ToolRegistry
|
||||
from openjarvis.core.types import ToolCall, ToolResult
|
||||
from openjarvis.core.types import Role, ToolCall, ToolResult
|
||||
from openjarvis.memory.store import LocalFactStore
|
||||
from openjarvis.tools._stubs import BaseTool, ToolSpec
|
||||
|
||||
@@ -195,6 +195,106 @@ class TestChatAgents:
|
||||
assert "simple ok" in result.output
|
||||
assert "failed" not in result.output.lower()
|
||||
|
||||
def test_agent_receives_prior_turn_history(self) -> None:
|
||||
"""Multi-turn chat must pass prior turns to agent.run() via AgentContext."""
|
||||
|
||||
captured_contexts: list[AgentContext | None] = []
|
||||
|
||||
class _CapturingAgent(BaseAgent):
|
||||
agent_id = "capturing_chat_agent"
|
||||
|
||||
def run(self, input, context: AgentContext | None = None, **kwargs):
|
||||
captured_contexts.append(context)
|
||||
return AgentResult(content=f"reply-{len(captured_contexts)}", turns=1)
|
||||
|
||||
engine = MagicMock()
|
||||
engine.engine_id = "mock"
|
||||
config = JarvisConfig()
|
||||
config.intelligence.default_model = "test-model"
|
||||
|
||||
AgentRegistry.register_value("capturing_chat_agent", _CapturingAgent)
|
||||
|
||||
with (
|
||||
patch("openjarvis.cli.chat_cmd.load_config", return_value=config),
|
||||
patch("openjarvis.engine.get_engine", return_value=("mock", engine)),
|
||||
patch("openjarvis.intelligence.register_builtin_models"),
|
||||
):
|
||||
result = CliRunner().invoke(
|
||||
chat,
|
||||
["--agent", "capturing_chat_agent", "--model", "test-model"],
|
||||
input="first turn\nsecond turn\n/quit\n",
|
||||
)
|
||||
|
||||
assert result.exit_code == 0
|
||||
assert len(captured_contexts) == 2
|
||||
|
||||
first_turn_context, second_turn_context = captured_contexts
|
||||
assert first_turn_context is not None
|
||||
assert first_turn_context.conversation.messages == []
|
||||
|
||||
assert second_turn_context is not None
|
||||
prior_texts = [m.content for m in second_turn_context.conversation.messages]
|
||||
assert "first turn" in prior_texts
|
||||
assert "reply-1" in prior_texts
|
||||
|
||||
def test_agent_memory_context_precedes_prior_turn_history(self, tmp_path) -> None:
|
||||
"""Memory system context must remain ahead of prior conversation turns."""
|
||||
|
||||
captured_contexts: list[AgentContext | None] = []
|
||||
|
||||
class _CapturingAgent(BaseAgent):
|
||||
agent_id = "capturing_memory_chat_agent"
|
||||
|
||||
def run(self, input, context: AgentContext | None = None, **kwargs):
|
||||
captured_contexts.append(context)
|
||||
return AgentResult(content=f"reply-{len(captured_contexts)}", turns=1)
|
||||
|
||||
facts_path = tmp_path / "facts.jsonl"
|
||||
LocalFactStore(facts_path).add("The user likes jazz", source="auto")
|
||||
|
||||
engine = MagicMock()
|
||||
engine.engine_id = "mock"
|
||||
config = JarvisConfig()
|
||||
config.intelligence.default_model = "test-model"
|
||||
config.memory.enabled = True
|
||||
config.memory.facts_path = str(facts_path)
|
||||
config.agent.context_from_memory = True
|
||||
|
||||
AgentRegistry.register_value(
|
||||
"capturing_memory_chat_agent",
|
||||
_CapturingAgent,
|
||||
)
|
||||
|
||||
with (
|
||||
patch("openjarvis.cli.chat_cmd.load_config", return_value=config),
|
||||
patch("openjarvis.engine.get_engine", return_value=("mock", engine)),
|
||||
patch("openjarvis.intelligence.register_builtin_models"),
|
||||
patch("openjarvis.memory.build_memory_service", return_value=None),
|
||||
patch("openjarvis.cli.ask._get_memory_backend", return_value=None),
|
||||
):
|
||||
result = CliRunner().invoke(
|
||||
chat,
|
||||
["--agent", "capturing_memory_chat_agent", "--model", "test-model"],
|
||||
input="first turn\nsecond turn\n/quit\n",
|
||||
)
|
||||
|
||||
assert result.exit_code == 0
|
||||
assert len(captured_contexts) == 2
|
||||
|
||||
second_turn_context = captured_contexts[1]
|
||||
assert second_turn_context is not None
|
||||
messages = second_turn_context.conversation.messages
|
||||
assert [message.role for message in messages] == [
|
||||
Role.SYSTEM,
|
||||
Role.USER,
|
||||
Role.ASSISTANT,
|
||||
]
|
||||
assert "user likes jazz" in messages[0].content
|
||||
assert [message.content for message in messages[1:]] == [
|
||||
"first turn",
|
||||
"reply-1",
|
||||
]
|
||||
|
||||
def test_memory_service_started_fed_and_stopped(self) -> None:
|
||||
"""The REPL starts memory, publishes each turn, and stops it."""
|
||||
|
||||
|
||||
@@ -0,0 +1,129 @@
|
||||
"""Regression: ``jarvis serve`` must wire the ``SystemPromptBuilder`` into the
|
||||
agent it constructs, so SOUL.md / MEMORY.md / USER.md reach the model over HTTP.
|
||||
|
||||
``cli/ask.py`` (and ``cli/chat_cmd.py`` and the managed-agent executor) have
|
||||
wired the builder since the persona system landed. The serve path never did, so
|
||||
an agent served over HTTP silently answered as a generic assistant — explicitly
|
||||
denying the persona — while the same agent via the CLI kept it. Found deploying
|
||||
a personal assistant: SOUL.md was correct on disk the whole time; no error, no
|
||||
warning.
|
||||
|
||||
This test boots ``serve`` just far enough to capture the agent handed to
|
||||
``create_app`` and asserts the builder (and thus the persona content) is
|
||||
present. It fails on the unpatched serve path: ``agent._prompt_builder`` is
|
||||
``None``, so the persona files never reach the model.
|
||||
"""
|
||||
|
||||
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 a package attribute resolves to the click
|
||||
# *command* (re-exported); 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"
|
||||
engine.engine_id = "mock"
|
||||
return engine
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"agent_name",
|
||||
["simple", "orchestrator", "monitor_operative", "operative"],
|
||||
)
|
||||
def test_serve_wires_persona_builder_into_served_agent(
|
||||
tmp_path, monkeypatch, agent_name
|
||||
):
|
||||
"""The agent built on the serve path must carry a SystemPromptBuilder whose
|
||||
assembled prompt includes SOUL.md content (regression for the HTTP persona
|
||||
loss)."""
|
||||
from openjarvis.agents.monitor_operative import MonitorOperativeAgent
|
||||
from openjarvis.agents.operative import OperativeAgent
|
||||
from openjarvis.agents.orchestrator import OrchestratorAgent
|
||||
from openjarvis.agents.simple import SimpleAgent
|
||||
from openjarvis.core.config import JarvisConfig
|
||||
from openjarvis.core.registry import AgentRegistry
|
||||
|
||||
# Persona file with a unique sentinel we can grep for in the built prompt.
|
||||
soul = tmp_path / "SOUL.md"
|
||||
soul.write_text("SERVE_PERSONA_SENTINEL", encoding="utf-8")
|
||||
|
||||
# conftest clears registries per-test; re-register the agent we exercise.
|
||||
agent_classes = {
|
||||
"simple": SimpleAgent,
|
||||
"orchestrator": OrchestratorAgent,
|
||||
"monitor_operative": MonitorOperativeAgent,
|
||||
"operative": OperativeAgent,
|
||||
}
|
||||
if not AgentRegistry.contains(agent_name):
|
||||
AgentRegistry.register_value(agent_name, agent_classes[agent_name])
|
||||
|
||||
config = JarvisConfig()
|
||||
config.server.host = "127.0.0.1"
|
||||
config.server.port = 8123
|
||||
config.intelligence.default_model = "test-model"
|
||||
config.memory_files.soul_path = str(soul)
|
||||
# Keep the heavy optional subsystems off so we reach create_app cleanly.
|
||||
config.telemetry.enabled = False
|
||||
config.agent_manager.enabled = False
|
||||
config.sessions.enabled = False
|
||||
config.channel.enabled = False
|
||||
config.skills.enabled = False
|
||||
config.agent.context_from_memory = False
|
||||
|
||||
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.
|
||||
sec = MagicMock()
|
||||
sec.engine = engine
|
||||
sec.capability_policy = None
|
||||
sec.audit_logger = None
|
||||
monkeypatch.setattr("openjarvis.security.setup_security", lambda *a, **k: sec)
|
||||
|
||||
captured: dict = {}
|
||||
|
||||
def _capture_create_app(*args, **kwargs):
|
||||
captured["agent"] = kwargs.get("agent")
|
||||
return MagicMock(name="app")
|
||||
|
||||
with (
|
||||
patch("openjarvis.server.app.create_app", side_effect=_capture_create_app),
|
||||
patch("uvicorn.run", lambda *a, **k: None),
|
||||
):
|
||||
result = CliRunner().invoke(
|
||||
cli, ["serve", "--agent", agent_name], catch_exceptions=False
|
||||
)
|
||||
|
||||
assert result.exit_code == 0, result.output
|
||||
agent = captured.get("agent")
|
||||
assert agent is not None, (
|
||||
"serve did not construct an agent or never reached create_app; "
|
||||
f"output:\n{result.output}"
|
||||
)
|
||||
# The regression: without the fix ``agent._prompt_builder`` is None and the
|
||||
# persona files never reach the model over HTTP.
|
||||
assert agent._prompt_builder is not None, (
|
||||
f"serve constructed {agent_name} without a prompt_builder — SOUL.md / "
|
||||
"MEMORY.md / USER.md would be silently dropped on the HTTP path."
|
||||
)
|
||||
assert "SERVE_PERSONA_SENTINEL" in agent._prompt_builder.build(), (
|
||||
"prompt_builder is wired on serve, but its built prompt omits SOUL.md"
|
||||
)
|
||||
@@ -95,6 +95,30 @@ class TestJarvisAsk:
|
||||
assert result == "Agent response"
|
||||
j.close()
|
||||
|
||||
def test_ask_with_agent_wires_persona(self, tmp_path):
|
||||
from openjarvis.agents.simple import SimpleAgent
|
||||
from openjarvis.core.registry import AgentRegistry
|
||||
|
||||
soul = tmp_path / "SOUL.md"
|
||||
soul.write_text("SDK_PERSONA_SENTINEL", encoding="utf-8")
|
||||
|
||||
cfg = JarvisConfig()
|
||||
cfg.memory_files.soul_path = str(soul)
|
||||
cfg.memory_files.memory_path = ""
|
||||
cfg.memory_files.user_path = ""
|
||||
cfg.agent.context_from_memory = False
|
||||
|
||||
if not AgentRegistry.contains("simple"):
|
||||
AgentRegistry.register_value("simple", SimpleAgent)
|
||||
|
||||
engine = _make_engine()
|
||||
with patch("openjarvis.sdk.get_engine", return_value=("mock", engine)):
|
||||
j = Jarvis(config=cfg, model="test-model")
|
||||
j.ask("Hello", agent="simple")
|
||||
messages = engine.generate.call_args.args[0]
|
||||
assert "SDK_PERSONA_SENTINEL" in messages[0].content
|
||||
j.close()
|
||||
|
||||
def test_ask_no_engine_raises(self):
|
||||
with patch("openjarvis.sdk.get_engine", return_value=None):
|
||||
j = Jarvis(config=JarvisConfig())
|
||||
|
||||
Reference in New Issue
Block a user