mirror of
https://github.com/open-jarvis/OpenJarvis.git
synced 2026-07-27 21:05:34 +00:00
fix(agents): persistent agents honor SOUL.md / MEMORY.md / USER.md persona files (#376)
SystemPromptBuilder (which loads the SOUL/MEMORY/USER persona files) was wired only into the one-shot `jarvis ask` path, so persistent agents run through AgentExecutor ignored persona entirely. The naive "pass prompt_builder" fix is insufficient: `prompt_builder` was dropped at every __init__ hop (ToolUsingAgent never accepted/forwarded it), and monitor_operative/operative assemble their own system prompt and never consult `_prompt_builder` — so they would silently ignore it even if it arrived. Fix: - `SystemPromptBuilder.persona_sections()` returns just the SOUL/MEMORY/USER sections (no agent template), for agents that build their own prompt and want to *append* persona rather than have it replace their instructions. - `BaseAgent._apply_persona()` appends persona to a self-assembled prompt (no-op without a builder or persona files). - Thread `prompt_builder` through the __init__ chain: ToolUsingAgent now accepts and forwards it to BaseAgent; monitor_operative and operative forward it and call `_apply_persona()` on their assembled system prompt. - AgentExecutor constructs a SystemPromptBuilder from config and passes it to any agent whose __init__ accepts it (same gating as session_store / memory_backend). Agents that override __init__ without forwarding (e.g. orchestrator) opt out automatically and keep their own machinery. Specialized prompts are preserved — persona is appended, not substituted. The one-shot path is unchanged. Verified: persona_sections() excludes the template but build() still includes both; monitor_operative/operative receive the builder through the chain and their assembled prompt includes the SOUL content. New tests in tests/agents/test_persona_persistent.py (11 passed; ruff clean). Closes #376 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.7
parent
00065cd960
commit
4ece9a933e
@@ -121,6 +121,23 @@ class BaseAgent(ABC):
|
||||
payload.update(data)
|
||||
self._bus.publish(EventType.AGENT_TURN_END, payload)
|
||||
|
||||
def _apply_persona(self, system_prompt: Optional[str]) -> Optional[str]:
|
||||
"""Append SOUL/MEMORY/USER persona to a self-assembled system prompt.
|
||||
|
||||
Agents like ``monitor_operative`` / ``operative`` build their own
|
||||
system prompt and bypass ``_build_messages`` (and thus the prompt
|
||||
builder). This lets them honor the same persona files as one-shot
|
||||
``jarvis ask`` (#376) by *appending* persona to — never replacing —
|
||||
their specialized instructions. No-op when no ``prompt_builder`` is
|
||||
wired or no persona files exist.
|
||||
"""
|
||||
if self._prompt_builder is None:
|
||||
return system_prompt
|
||||
persona = self._prompt_builder.persona_sections()
|
||||
if not persona:
|
||||
return system_prompt
|
||||
return f"{system_prompt}\n\n{persona}" if system_prompt else persona
|
||||
|
||||
def _build_messages(
|
||||
self,
|
||||
input: str,
|
||||
@@ -307,6 +324,7 @@ class ToolUsingAgent(BaseAgent):
|
||||
interactive: bool = False,
|
||||
confirm_callback: Optional[Any] = None,
|
||||
skill_few_shot_examples: Optional[List[str]] = None,
|
||||
prompt_builder: Optional[Any] = None,
|
||||
) -> None:
|
||||
super().__init__(
|
||||
engine,
|
||||
@@ -314,6 +332,7 @@ class ToolUsingAgent(BaseAgent):
|
||||
bus=bus,
|
||||
temperature=temperature,
|
||||
max_tokens=max_tokens,
|
||||
prompt_builder=prompt_builder,
|
||||
)
|
||||
from openjarvis.tools._stubs import ToolExecutor
|
||||
|
||||
|
||||
@@ -404,6 +404,21 @@ class AgentExecutor:
|
||||
state_kwargs["memory_backend"] = getattr(
|
||||
self._system, "memory_backend", None
|
||||
)
|
||||
# Wire SOUL.md / MEMORY.md / USER.md persona files into persistent
|
||||
# agents, mirroring the one-shot `jarvis ask` path so they no
|
||||
# longer apply to CLI calls only (#376).
|
||||
cfg = getattr(self._system, "config", None)
|
||||
if cfg is not None and _accepts("prompt_builder"):
|
||||
from openjarvis.prompt.builder import SystemPromptBuilder
|
||||
|
||||
state_kwargs["prompt_builder"] = SystemPromptBuilder(
|
||||
agent_template=getattr(
|
||||
cfg.agent, "default_system_prompt", ""
|
||||
)
|
||||
or "",
|
||||
memory_files_config=cfg.memory_files,
|
||||
system_prompt_config=cfg.system_prompt,
|
||||
)
|
||||
|
||||
try:
|
||||
agent_instance = agent_cls(
|
||||
|
||||
@@ -139,6 +139,7 @@ class MonitorOperativeAgent(ToolUsingAgent):
|
||||
max_tokens=max_tokens,
|
||||
interactive=interactive,
|
||||
confirm_callback=confirm_callback,
|
||||
prompt_builder=kwargs.get("prompt_builder"),
|
||||
)
|
||||
# Validate strategies
|
||||
if memory_extraction not in VALID_MEMORY_EXTRACTION:
|
||||
@@ -215,6 +216,9 @@ class MonitorOperativeAgent(ToolUsingAgent):
|
||||
sys_parts.append(f"\n## Previous State\n{previous_state}")
|
||||
|
||||
system_prompt = "\n\n".join(sys_parts) if sys_parts else None
|
||||
# Honor SOUL.md / MEMORY.md / USER.md persona files like `jarvis ask`,
|
||||
# appended so the monitor's own instructions are preserved (#376).
|
||||
system_prompt = self._apply_persona(system_prompt)
|
||||
|
||||
# 3. Load session history
|
||||
session_messages = self._load_session()
|
||||
|
||||
@@ -70,6 +70,7 @@ class OperativeAgent(ToolUsingAgent):
|
||||
max_tokens=max_tokens,
|
||||
interactive=interactive,
|
||||
confirm_callback=confirm_callback,
|
||||
prompt_builder=kwargs.get("prompt_builder"),
|
||||
)
|
||||
self._system_prompt = system_prompt or ""
|
||||
self._operator_id = operator_id
|
||||
@@ -96,6 +97,9 @@ class OperativeAgent(ToolUsingAgent):
|
||||
sys_parts.append(f"\n## Previous State\n{previous_state}")
|
||||
|
||||
system_prompt = "\n\n".join(sys_parts) if sys_parts else None
|
||||
# Honor SOUL.md / MEMORY.md / USER.md persona files like `jarvis ask`,
|
||||
# appended so the operative's own instructions are preserved (#376).
|
||||
system_prompt = self._apply_persona(system_prompt)
|
||||
|
||||
# 3. Load session history
|
||||
session_messages = self._load_session()
|
||||
|
||||
@@ -45,9 +45,9 @@ class SystemPromptBuilder:
|
||||
parts.append(f"\n\n## Previous State\n\n{self._previous_state}")
|
||||
return "".join(parts)
|
||||
|
||||
def _build_frozen_prefix(self) -> str:
|
||||
def _persona_sections(self) -> list[str]:
|
||||
"""The SOUL / MEMORY / USER sections (no agent template, no skills)."""
|
||||
sections: list[str] = []
|
||||
sections.append(self._agent_template)
|
||||
soul = self._load_file(
|
||||
self._mf_config.soul_path,
|
||||
self._sp_config.soul_max_chars,
|
||||
@@ -66,6 +66,22 @@ class SystemPromptBuilder:
|
||||
)
|
||||
if user:
|
||||
sections.append(f"## User Profile\n\n{user}")
|
||||
return sections
|
||||
|
||||
def persona_sections(self) -> str:
|
||||
"""Just the SOUL / MEMORY / USER persona, joined.
|
||||
|
||||
For agents that assemble their own system prompt (monitor_operative,
|
||||
operative) and want to *append* persona without letting the builder
|
||||
replace their specialized instructions (#376). Returns "" when no
|
||||
persona files are present.
|
||||
"""
|
||||
return "\n\n".join(self._persona_sections())
|
||||
|
||||
def _build_frozen_prefix(self) -> str:
|
||||
sections: list[str] = []
|
||||
sections.append(self._agent_template)
|
||||
sections.extend(self._persona_sections())
|
||||
# XML skill catalog (preferred over legacy markdown list)
|
||||
if self._skill_catalog_xml:
|
||||
sections.append("## Available Skills\n\n" + self._skill_catalog_xml)
|
||||
|
||||
@@ -0,0 +1,96 @@
|
||||
"""Persona files reach persistent agents, not just one-shot `jarvis ask` (#376).
|
||||
|
||||
SOUL.md / MEMORY.md / USER.md are loaded by `jarvis ask` via SystemPromptBuilder.
|
||||
Persistent agents (monitor_operative, operative) assemble their own system
|
||||
prompt and previously ignored these files entirely. These tests verify the
|
||||
persona is now appended to their prompt without replacing their specialized
|
||||
instructions.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from openjarvis.core.config import MemoryFilesConfig, SystemPromptConfig
|
||||
from openjarvis.prompt.builder import SystemPromptBuilder
|
||||
|
||||
|
||||
def _builder_with_soul(tmp_path, text="You are Kira."):
|
||||
soul = tmp_path / "SOUL.md"
|
||||
soul.write_text(text, encoding="utf-8")
|
||||
mf = MemoryFilesConfig(
|
||||
soul_path=str(soul),
|
||||
memory_path=str(tmp_path / "MEMORY.md"), # absent → ignored
|
||||
user_path=str(tmp_path / "USER.md"), # absent → ignored
|
||||
)
|
||||
return SystemPromptBuilder(
|
||||
agent_template="AGENT_TEMPLATE",
|
||||
memory_files_config=mf,
|
||||
system_prompt_config=SystemPromptConfig(),
|
||||
)
|
||||
|
||||
|
||||
class TestPersonaSections:
|
||||
def test_persona_sections_excludes_template(self, tmp_path):
|
||||
builder = _builder_with_soul(tmp_path)
|
||||
persona = builder.persona_sections()
|
||||
assert "You are Kira." in persona
|
||||
assert "AGENT_TEMPLATE" not in persona # template is NOT in persona
|
||||
|
||||
def test_full_build_still_includes_template_and_persona(self, tmp_path):
|
||||
builder = _builder_with_soul(tmp_path)
|
||||
full = builder.build()
|
||||
assert "AGENT_TEMPLATE" in full
|
||||
assert "You are Kira." in full
|
||||
|
||||
def test_persona_sections_empty_when_no_files(self, tmp_path):
|
||||
mf = MemoryFilesConfig(
|
||||
soul_path=str(tmp_path / "nope.md"),
|
||||
memory_path=str(tmp_path / "nope2.md"),
|
||||
user_path=str(tmp_path / "nope3.md"),
|
||||
)
|
||||
builder = SystemPromptBuilder(agent_template="T", memory_files_config=mf)
|
||||
assert builder.persona_sections() == ""
|
||||
|
||||
|
||||
class TestApplyPersona:
|
||||
def test_appends_to_base_prompt(self, tmp_path):
|
||||
from openjarvis.agents.simple import SimpleAgent
|
||||
|
||||
agent = SimpleAgent(object(), "m", prompt_builder=_builder_with_soul(tmp_path))
|
||||
out = agent._apply_persona("MONITOR INSTRUCTIONS")
|
||||
assert out.startswith("MONITOR INSTRUCTIONS")
|
||||
assert "You are Kira." in out
|
||||
|
||||
def test_noop_without_builder(self):
|
||||
from openjarvis.agents.simple import SimpleAgent
|
||||
|
||||
agent = SimpleAgent(object(), "m")
|
||||
assert agent._apply_persona("BASE") == "BASE"
|
||||
|
||||
|
||||
class TestPersistentAgentsReceivePromptBuilder:
|
||||
"""The __init__ chain must forward prompt_builder through to BaseAgent."""
|
||||
|
||||
def test_monitor_operative_forwards_prompt_builder(self, tmp_path):
|
||||
from openjarvis.agents.monitor_operative import MonitorOperativeAgent
|
||||
|
||||
builder = _builder_with_soul(tmp_path)
|
||||
agent = MonitorOperativeAgent(object(), "m", prompt_builder=builder)
|
||||
assert agent._prompt_builder is builder
|
||||
applied = agent._apply_persona("MONITOR INSTRUCTIONS")
|
||||
assert "MONITOR INSTRUCTIONS" in applied
|
||||
assert "You are Kira." in applied
|
||||
|
||||
def test_operative_forwards_prompt_builder(self, tmp_path):
|
||||
from openjarvis.agents.operative import OperativeAgent
|
||||
|
||||
builder = _builder_with_soul(tmp_path)
|
||||
agent = OperativeAgent(object(), "m", prompt_builder=builder)
|
||||
assert agent._prompt_builder is builder
|
||||
assert "You are Kira." in agent._apply_persona("OP INSTRUCTIONS")
|
||||
|
||||
def test_monitor_operative_without_builder_is_unaffected(self):
|
||||
from openjarvis.agents.monitor_operative import MonitorOperativeAgent
|
||||
|
||||
agent = MonitorOperativeAgent(object(), "m")
|
||||
assert agent._prompt_builder is None
|
||||
assert agent._apply_persona("BASE") == "BASE"
|
||||
Reference in New Issue
Block a user