From 09b19193fe548973d8f28357de91e2d0ffd55154 Mon Sep 17 00:00:00 2001 From: Arush Wadhawan Date: Mon, 1 Jun 2026 14:31:19 -0400 Subject: [PATCH] fix(server): load SOUL.md / USER.md context in streaming chat (#449) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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) --------- Co-authored-by: Jon Saad-Falcon <41205309+jonsaadfalcon@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) --- src/openjarvis/server/agent_manager_routes.py | 39 +++- tests/server/test_managed_agent_streaming.py | 185 ++++++++++++++---- 2 files changed, 185 insertions(+), 39 deletions(-) diff --git a/src/openjarvis/server/agent_manager_routes.py b/src/openjarvis/server/agent_manager_routes.py index 91459ba7..9c2da535 100644 --- a/src/openjarvis/server/agent_manager_routes.py +++ b/src/openjarvis/server/agent_manager_routes.py @@ -439,6 +439,26 @@ _SAMPLER_PARAM_KEYS = ( ) +def _build_managed_system_prompt(system_prompt: str, app_config: Any) -> str: + """Build the streaming managed-agent system prompt via SystemPromptBuilder. + + Routes the agent's own ``system_prompt`` through the same builder the + CLI/ask path uses, so SOUL.md / MEMORY.md / USER.md persona files are + injected for streaming chat too (#431). Returns the assembled prompt + (caller decides whether to append a SYSTEM message); an agent with + neither persona nor template yields an empty string, preserving the + prior no-SYSTEM-message behavior. + """ + from openjarvis.prompt.builder import SystemPromptBuilder + + builder = SystemPromptBuilder( + agent_template=system_prompt or "", + memory_files_config=getattr(app_config, "memory_files", None), + system_prompt_config=getattr(app_config, "system_prompt", None), + ) + return builder.build() + + def _sampler_kwargs(config: Dict[str, Any]) -> Dict[str, Any]: """Extract per-agent sampler params from a managed agent's config (#386).""" out: Dict[str, Any] = {} @@ -811,8 +831,23 @@ async def _stream_managed_agent( # Build conversation messages from history + current input llm_messages: List[Message] = [] - if system_prompt: - llm_messages.append(Message(role=Role.SYSTEM, content=system_prompt)) + + # Wire the SystemPromptBuilder to inject SOUL.md / MEMORY.md / USER.md + # persona files (parity with the CLI/ask path) — see #431. + app_config = getattr(app_state, "config", None) + if app_config is None: + from openjarvis.core.config import load_config + + app_config = load_config() + + final_system_prompt = _build_managed_system_prompt( + system_prompt or "", app_config + ) + + if final_system_prompt and final_system_prompt.strip(): + llm_messages.append( + Message(role=Role.SYSTEM, content=final_system_prompt.strip()) + ) # Resolve agent type and class for DeepResearch tool wiring agent_type = agent_record.get("agent_type", "") diff --git a/tests/server/test_managed_agent_streaming.py b/tests/server/test_managed_agent_streaming.py index cf57047b..143c7d3e 100644 --- a/tests/server/test_managed_agent_streaming.py +++ b/tests/server/test_managed_agent_streaming.py @@ -15,6 +15,7 @@ pytest.importorskip("fastapi") # agent_manager_routes imports FastAPI at module from openjarvis.core.types import Role # noqa: E402 from openjarvis.server.agent_manager_routes import ( # noqa: E402 + _build_managed_system_prompt, _instantiate_managed_tool, _replay_history_messages, _sampler_kwargs, @@ -26,12 +27,26 @@ class TestReplayHistoryToolCalls: def test_assistant_tool_calls_are_replayed_with_results(self): history = [ # DESC order (newest first), as list_messages returns - {"id": "m2", "direction": "agent_to_user", "content": "", "tool_calls": [ - {"tool": "shell_exec", "arguments": '{"command":"pwd"}', - "result": "/home/u", "success": True, "latency": 1.0}, - ]}, - {"id": "m1", "direction": "user_to_agent", "content": "run pwd", - "tool_calls": None}, + { + "id": "m2", + "direction": "agent_to_user", + "content": "", + "tool_calls": [ + { + "tool": "shell_exec", + "arguments": '{"command":"pwd"}', + "result": "/home/u", + "success": True, + "latency": 1.0, + }, + ], + }, + { + "id": "m1", + "direction": "user_to_agent", + "content": "run pwd", + "tool_calls": None, + }, ] msgs = _replay_history_messages(history, exclude_id="current") # Chronological: user, assistant(tool_calls), tool(result) @@ -46,8 +61,12 @@ class TestReplayHistoryToolCalls: def test_plain_assistant_without_tool_calls(self): history = [ - {"id": "m1", "direction": "agent_to_user", "content": "hi", - "tool_calls": None}, + { + "id": "m1", + "direction": "agent_to_user", + "content": "hi", + "tool_calls": None, + }, ] msgs = _replay_history_messages(history, exclude_id="current") assert len(msgs) == 1 @@ -56,10 +75,18 @@ class TestReplayHistoryToolCalls: def test_excludes_current_message(self): history = [ - {"id": "cur", "direction": "user_to_agent", "content": "now", - "tool_calls": None}, - {"id": "old", "direction": "user_to_agent", "content": "before", - "tool_calls": None}, + { + "id": "cur", + "direction": "user_to_agent", + "content": "now", + "tool_calls": None, + }, + { + "id": "old", + "direction": "user_to_agent", + "content": "before", + "tool_calls": None, + }, ] msgs = _replay_history_messages(history, exclude_id="cur") assert [m.content for m in msgs] == ["before"] @@ -69,13 +96,24 @@ class TestSamplerKwargs: """#386 — sampler params forwarded only when set.""" def test_reads_present_keys(self): - cfg = {"temperature": 0.7, "repetition_penalty": 1.1, "top_p": 0.9, - "top_k": 40, "min_p": 0.05, "frequency_penalty": 0.2, - "presence_penalty": 0.1} + cfg = { + "temperature": 0.7, + "repetition_penalty": 1.1, + "top_p": 0.9, + "top_k": 40, + "min_p": 0.05, + "frequency_penalty": 0.2, + "presence_penalty": 0.1, + } out = _sampler_kwargs(cfg) - assert out == {"repetition_penalty": 1.1, "top_p": 0.9, "top_k": 40, - "min_p": 0.05, "frequency_penalty": 0.2, - "presence_penalty": 0.1} + assert out == { + "repetition_penalty": 1.1, + "top_p": 0.9, + "top_k": 40, + "min_p": 0.05, + "frequency_penalty": 0.2, + "presence_penalty": 0.1, + } # temperature/max_tokens are handled separately, not here. assert "temperature" not in out @@ -98,34 +136,107 @@ class TestInstantiateManagedTool: def test_memory_tool_gets_backend(self): backend = object() - app_state = SimpleNamespace(memory_backend=backend, config=None, - channel_bridge=None) - tool = _instantiate_managed_tool(_FakeTool, "memory_store", - engine=object(), model="m", - app_state=app_state) + app_state = SimpleNamespace( + memory_backend=backend, config=None, channel_bridge=None + ) + tool = _instantiate_managed_tool( + _FakeTool, "memory_store", engine=object(), model="m", app_state=app_state + ) assert tool.kwargs == {"backend": backend} def test_llm_tool_gets_engine_and_model(self): engine = object() - app_state = SimpleNamespace(memory_backend=None, config=None, - channel_bridge=None) - tool = _instantiate_managed_tool(_FakeTool, "llm", engine=engine, - model="qwen", app_state=app_state) + app_state = SimpleNamespace( + memory_backend=None, config=None, channel_bridge=None + ) + tool = _instantiate_managed_tool( + _FakeTool, "llm", engine=engine, model="qwen", app_state=app_state + ) assert tool.kwargs == {"engine": engine, "model": "qwen"} def test_channel_tool_gets_channel(self): bridge = object() - app_state = SimpleNamespace(memory_backend=None, config=None, - channel_bridge=bridge) - tool = _instantiate_managed_tool(_FakeTool, "channel_send", - engine=object(), model="m", - app_state=app_state) + app_state = SimpleNamespace( + memory_backend=None, config=None, channel_bridge=bridge + ) + tool = _instantiate_managed_tool( + _FakeTool, "channel_send", engine=object(), model="m", app_state=app_state + ) assert tool.kwargs == {"channel": bridge} def test_plain_tool_gets_no_injection(self): - app_state = SimpleNamespace(memory_backend=None, config=None, - channel_bridge=None) - tool = _instantiate_managed_tool(_FakeTool, "calculator", - engine=object(), model="m", - app_state=app_state) + app_state = SimpleNamespace( + memory_backend=None, config=None, channel_bridge=None + ) + tool = _instantiate_managed_tool( + _FakeTool, "calculator", engine=object(), model="m", app_state=app_state + ) assert tool.kwargs == {} + + +class TestBuildManagedSystemPrompt: + """#431 — the streaming managed-agent path must run the agent's + system_prompt through SystemPromptBuilder so SOUL.md / MEMORY.md / + USER.md persona files are injected (parity with the CLI/ask path), + instead of using the raw config system_prompt verbatim. + """ + + def _config_with_soul(self, tmp_path): + from openjarvis.core.config import ( + MemoryFilesConfig, + SystemPromptConfig, + ) + + soul = tmp_path / "SOUL.md" + soul.write_text("You are Jarvis, a meticulous local-first assistant.") + # Other persona files point at non-existent paths — only SOUL is set. + return SimpleNamespace( + memory_files=MemoryFilesConfig(soul_path=str(soul)), + system_prompt=SystemPromptConfig(), + ) + + def test_injects_soul_persona(self, tmp_path): + app_config = self._config_with_soul(tmp_path) + result = _build_managed_system_prompt( + system_prompt="You are a helpful assistant.", + app_config=app_config, + ) + # The persona file content is injected (the #431 fix)... + assert "meticulous local-first assistant" in result + # ...alongside the agent's own template. + assert "You are a helpful assistant." in result + + def test_agent_template_is_preserved(self, tmp_path): + from openjarvis.core.config import MemoryFilesConfig, SystemPromptConfig + + # No persona files configured (all default paths). + app_config = SimpleNamespace( + memory_files=MemoryFilesConfig(), + system_prompt=SystemPromptConfig(), + ) + result = _build_managed_system_prompt( + system_prompt="Plain agent.", + app_config=app_config, + ) + # The agent's own template is carried into the assembled prompt. + assert "Plain agent." in result + + def test_matches_cli_builder_output(self, tmp_path): + """Parity check: the helper produces exactly what a directly- + constructed SystemPromptBuilder produces (same path the CLI uses), + so streaming chat and `jarvis ask` assemble the prompt identically. + """ + from openjarvis.core.config import MemoryFilesConfig, SystemPromptConfig + from openjarvis.prompt.builder import SystemPromptBuilder + + app_config = SimpleNamespace( + memory_files=MemoryFilesConfig(), + system_prompt=SystemPromptConfig(), + ) + helper_out = _build_managed_system_prompt("Agent X.", app_config) + direct_out = SystemPromptBuilder( + agent_template="Agent X.", + memory_files_config=app_config.memory_files, + system_prompt_config=app_config.system_prompt, + ).build() + assert helper_out == direct_out