diff --git a/src/openjarvis/cli/ask.py b/src/openjarvis/cli/ask.py index 165d5f68..06782b8a 100644 --- a/src/openjarvis/cli/ask.py +++ b/src/openjarvis/cli/ask.py @@ -357,6 +357,22 @@ def _run_agent( if capability_policy is not None: agent_kwargs["capability_policy"] = capability_policy + # 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). + 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) ctx = AgentContext() diff --git a/src/openjarvis/prompt/builder.py b/src/openjarvis/prompt/builder.py index 1b562fb0..f014c3a7 100644 --- a/src/openjarvis/prompt/builder.py +++ b/src/openjarvis/prompt/builder.py @@ -86,7 +86,10 @@ class SystemPromptBuilder: path = Path(path_str).expanduser() if not path.exists(): return "" - content = path.read_text() + # Always read as UTF-8. On Windows, ``read_text()`` falls back to the + # system code page (e.g. cp950 for zh-TW, cp932 for ja) and raises + # ``UnicodeDecodeError`` on any non-ASCII persona content. + content = path.read_text(encoding="utf-8") if len(content) <= max_chars: return content return self._truncate(content, max_chars) diff --git a/tests/cli/test_ask_agent.py b/tests/cli/test_ask_agent.py index 8da0e234..a1c4358f 100644 --- a/tests/cli/test_ask_agent.py +++ b/tests/cli/test_ask_agent.py @@ -336,3 +336,100 @@ class TestBuildTools: config = JarvisConfig() tools = _build_tools(["calculator", "think"], config, mock_setup, "test-model") assert len(tools) == 2 + + +class TestPersonaFilesReachModel: + """End-to-end coverage for SOUL.md / MEMORY.md / USER.md integration. + + Without these tests, the ``SystemPromptBuilder`` and the persona files + documented in the README are present in the codebase but never reach + the model — the bug this suite is meant to prevent regressing into. + """ + + def test_soul_md_content_reaches_engine_in_simple_agent( + self, runner, monkeypatch, tmp_path + ): + """SOUL.md content must appear in the system message sent to the engine.""" + from openjarvis.core.config import JarvisConfig + + # Write a SOUL.md with a unique sentinel string we can grep for + soul = tmp_path / "SOUL.md" + soul.write_text("PERSONA_SENTINEL_zh_jarvis", encoding="utf-8") + memory = tmp_path / "MEMORY.md" + memory.write_text("MEMORY_SENTINEL", encoding="utf-8") + user = tmp_path / "USER.md" + user.write_text("USER_SENTINEL", encoding="utf-8") + + cfg = JarvisConfig() + cfg.memory_files.soul_path = str(soul) + cfg.memory_files.memory_path = str(memory) + cfg.memory_files.user_path = str(user) + cfg.agent.default_system_prompt = "BASELINE_TEMPLATE" + # Disable memory context injection so it doesn't add another SYSTEM + # message and confuse the assertion. + cfg.agent.context_from_memory = False + + engine = _mock_engine() + _register_agents() + _register_tools() + with ( + patch.object(_ask_mod, "load_config", return_value=cfg), + patch.object( + _ask_mod, "get_engine", return_value=("mock", engine) + ), + patch.object(_ask_mod, "discover_engines", return_value=[("mock", engine)]), + patch.object( + _ask_mod, "discover_models", return_value={"mock": ["test-model"]} + ), + patch.object(_ask_mod, "register_builtin_models"), + patch.object(_ask_mod, "merge_discovered_models"), + ): + result = runner.invoke(cli, ["ask", "--agent", "simple", "Hello"]) + + assert result.exit_code == 0, result.output + # Grab the messages passed to engine.generate + engine.generate.assert_called() + call_args = engine.generate.call_args + messages = call_args.args[0] if call_args.args else call_args.kwargs.get("messages") + assert messages is not None and len(messages) >= 2 + system_messages = [m for m in messages if str(m.role).endswith("SYSTEM")] + assert system_messages, f"No SYSTEM message in {messages!r}" + joined = "\n".join(m.content for m in system_messages) + assert "BASELINE_TEMPLATE" in joined + assert "PERSONA_SENTINEL_zh_jarvis" in joined + assert "MEMORY_SENTINEL" in joined + assert "USER_SENTINEL" in joined + + def test_orchestrator_keeps_its_own_system_prompt( + self, runner, monkeypatch, tmp_path + ): + """OrchestratorAgent's __init__ doesn't accept ``prompt_builder``; + the wiring must skip it silently rather than crash.""" + from openjarvis.core.config import JarvisConfig + + soul = tmp_path / "SOUL.md" + soul.write_text("ORCH_PERSONA_SENTINEL", encoding="utf-8") + + cfg = JarvisConfig() + cfg.memory_files.soul_path = str(soul) + cfg.agent.context_from_memory = False + + engine = _mock_engine() + _register_agents() + _register_tools() + with ( + patch.object(_ask_mod, "load_config", return_value=cfg), + patch.object( + _ask_mod, "get_engine", return_value=("mock", engine) + ), + patch.object(_ask_mod, "discover_engines", return_value=[("mock", engine)]), + patch.object( + _ask_mod, "discover_models", return_value={"mock": ["test-model"]} + ), + patch.object(_ask_mod, "register_builtin_models"), + patch.object(_ask_mod, "merge_discovered_models"), + ): + 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