Compare commits

...
Author SHA1 Message Date
aec96f9d5d fix(cli): pass prior conversation turns to agent.run() in chat REPL (#744)
* fix(cli): pass prior conversation turns to agent.run() in chat REPL

Why: jarvis chat built an AgentContext seeded only with an optional
memory-injected fact, never with the turn-by-turn conversation history
already tracked in `history`. Every agent-backed chat turn after the
first ran with no memory of what was said before it.

- src/openjarvis/cli/chat_cmd.py: always build AgentContext for
  agent-backed turns, seeded from prior non-system history messages,
  still layering the memory-fact message on top when present
- tests/cli/test_chat_cmd.py: regression test asserting the second
  turn's AgentContext carries the first turn's user/assistant messages

* fix(cli): keep memory context before chat history

---------

Co-authored-by: Ari <ari.silva@paipe.co>
Co-authored-by: Elliot Slusky <elliot@slusky.com>
2026-08-14 17:25:23 -07:00
Elliot Slusky c2e1c375aa Merge pull request #679 from gilbert-barajas/fix/serve-persona-prompt-builder
fix: served/SDK agents silently lose their persona (SOUL.md never reaches the model over HTTP)
2026-08-14 17:16:19 -07:00
Elliot Slusky ef28e5f84b fix: complete persona wiring across agent entry points 2026-08-14 17:11:25 -07:00
Elliot Slusky 156d41d2f9 Merge remote-tracking branch 'origin/main' into elliot/pr679-fix 2026-08-14 17:02:52 -07:00
Gilbert Barajas 6e40d87eb5 fix: wire persona prompt_builder into the serve + SDK agent paths
cli/serve.py and sdk.py constructed their agents without passing
prompt_builder, so an agent reached over HTTP (jarvis serve) or via the
SDK silently lost its SOUL.md / MEMORY.md / USER.md persona while the
same agent via `jarvis ask` / `jarvis chat` kept it. cli/ask.py,
cli/chat_cmd.py, and the managed-agent executor already wired the
builder; these two entry points never did.

Found deploying a personal assistant: the server answered as a generic
assistant that explicitly denied being the persona, with SOUL.md
sitting correctly on disk the whole time. No error, no warning.

Mirrors the existing inspect.signature-guarded wiring from ask.py, so
agents whose __init__ doesn't accept the kwarg (e.g. OrchestratorAgent)
opt out automatically and keep their own system-prompt machinery.

Adds a serve-path regression test (tests/cli/test_serve_persona.py)
that fails without the fix: agent._prompt_builder is None on the
unpatched serve path, so the persona files never reach the model.
2026-07-27 00:50:56 -05:00
10 changed files with 305 additions and 14 deletions
+2 -1
View File
@@ -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:
+2 -1
View File
@@ -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
+2 -3
View File
@@ -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:
+6 -4
View File
@@ -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)
+21
View File
@@ -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).
+14
View File
@@ -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()
+4 -4
View File
@@ -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
View File
@@ -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."""
+129
View File
@@ -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"
)
+24
View File
@@ -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())