fix(agent-server): propagate load_memory preference to all launch paths

.pr/verify_load_memory_all_paths.py

@@ -0,0 +1,141 @@
+"""Manual, less-mocked verification for #4542.
+
+Proves, against real file I/O instead of pytest mocks:
+  Part A - load_memory() actually reads a real MEMORY.md from disk.
+  Part B - a real, on-disk load_memory=True preference reaches the launched
+           agent for both previously-broken shapes, `agent` and
+           `agent_settings`. `agent_profile_id` is not re-checked here -
+           that path was already fixed by #4223 and is covered by the
+           existing pytest suite.
+
+Run with: uv run python .pr/verify_load_memory_all_paths.py
+Requires OH_PERSISTENCE_DIR set first - without it this would write to your
+real ~/.openhands directory.
+"""
+
+import asyncio
+import os
+import tempfile
+from datetime import UTC, datetime
+from pathlib import Path
+from unittest.mock import AsyncMock, MagicMock, patch
+from uuid import uuid4
+
+from openhands.agent_server.conversation_service import ConversationService
+from openhands.agent_server.event_service import EventService
+from openhands.agent_server.models import StartConversationRequest
+from openhands.agent_server.persistence import (
+    PersistedSettings,
+    get_settings_store,
+    reset_stores,
+)
+from openhands.sdk import LLM, Agent, AgentContext
+from openhands.sdk.context.memory import load_memory
+from openhands.sdk.conversation.state import (
+    ConversationExecutionStatus,
+    ConversationState,
+)
+from openhands.sdk.settings.model import OpenHandsAgentSettings
+from openhands.sdk.workspace import LocalWorkspace
+
+
+def make_agent() -> Agent:
+    return Agent(llm=LLM(model="gpt-4o", usage_id="llm"), tools=[])
+
+
+async def launch_and_get_agent(tmp_path: Path, **request_kwargs):
+    """Launch through the real ConversationService; stub only the expensive
+    last-mile (actual agent execution / LLM call), never the settings store."""
+    request = StartConversationRequest(
+        workspace=LocalWorkspace(working_dir=str(tmp_path)),
+        **request_kwargs,
+    )
+    captured = {}
+
+    async def capture_start(_stored, **kwargs):
+        agent = kwargs["agent"]
+        captured["agent"] = agent
+        es = AsyncMock(spec=EventService)
+        es.get_state.return_value = ConversationState(
+            id=uuid4(),
+            agent=agent,
+            workspace=request.workspace,
+            execution_status=ConversationExecutionStatus.IDLE,
+        )
+        es.stored = MagicMock(
+            launched_agent_profile=None,
+            client_tools=[],
+            title=None,
+            metrics=None,
+            created_at=datetime.now(UTC),
+            updated_at=datetime.now(UTC),
+            forked_from_conversation_id=None,
+            forked_from_event_id=None,
+            parent_conversation_id=None,
+        )
+        return es
+
+    service = ConversationService(conversations_dir=tmp_path)
+    service._event_services = {}
+    with patch.object(
+        service,
+        "_start_event_service",
+        new_callable=AsyncMock,
+        side_effect=capture_start,
+    ):
+        await service.start_conversation(request)
+    return captured["agent"]
+
+
+async def main():
+    persistence_dir = Path(os.environ["OH_PERSISTENCE_DIR"])
+    reset_stores()  # safety net; harmless if there was nothing to reset
+    get_settings_store().save(
+        PersistedSettings(
+            agent_settings=OpenHandsAgentSettings(
+                agent_context=AgentContext(load_memory=True)
+            )
+        )
+    )
+    print(
+        f"Persisted load_memory=True for real at {persistence_dir / 'settings.json'}\n"
+    )
+
+    with tempfile.TemporaryDirectory() as workspace_dir:
+        workspace = Path(workspace_dir)
+        memory_dir = workspace / ".openhands" / "memory"
+        memory_dir.mkdir(parents=True)
+        (memory_dir / "MEMORY.md").write_text(
+            "# Verification note\n"
+            "load_memory propagated correctly if you can read this.\n"
+        )
+
+        print("=== Part A: does load_memory() read the file back? ===")
+        content = load_memory(workspace)
+        print(content)
+        assert content is not None and "propagated correctly" in content
+        print("PASS\n")
+
+        print("=== Part B: does the real settings store reach the launched agent? ===")
+        for label, kwargs in [
+            ("agent", {"agent": make_agent()}),
+            (
+                "agent_settings",
+                {
+                    "agent_settings": {
+                        "agent_kind": "openhands",
+                        "llm": {"model": "gpt-4o", "usage_id": "llm"},
+                    }
+                },
+            ),
+        ]:
+            agent = await launch_and_get_agent(workspace, **kwargs)
+            ok = agent.agent_context is not None and agent.agent_context.load_memory
+            print(f"{label:15s} -> agent_context.load_memory = {ok}")
+            assert ok, f"{label} did not inherit the persisted preference"
+
+    print("\nAll checks passed.")
+
+
+if __name__ == "__main__":
+    asyncio.run(main())

openhands-agent-server/openhands/agent_server/conversation_service.py

@@ -129,6 +129,19 @@ def _append_system_message_suffix(agent: AgentBase, addition: str) -> AgentBase:
     return agent.model_copy(update={"agent_context": updated_context})
 
 
+def _with_load_memory(agent: AgentBase) -> AgentBase:
+    """Stamp the global persistent-memory preference onto an agent.
+
+    ``load_memory`` is a user-level setting, not part of any agent, profile or
+    client payload, so it is applied here regardless of how the agent reached
+    the request.
+    """
+    context = agent.agent_context or AgentContext()
+    return agent.model_copy(
+        update={"agent_context": context.model_copy(update={"load_memory": True})}
+    )
+
+
 def _has_git_remote(repo_root: Path, remote: str = "origin") -> bool:
     try:
         run_git_command(["git", "remote", "get-url", remote], repo_root)
@@ -293,7 +306,6 @@ def _resolve_agent_from_profile(
     profile_id: "UUID",
     cipher: "Cipher | None",
     mcp_config: "dict[str, MCPServer]",
-    load_memory: bool = False,
 ) -> "tuple[AgentBase, LaunchedAgentProfile]":
     """Load and resolve an agent profile by id, returning the built agent + provenance.
 
@@ -304,11 +316,6 @@ def _resolve_agent_from_profile(
             server's cipher.  Passed explicitly so this free function never
             touches the settings-store singleton (which may not have been
             initialised with the correct cipher yet).
-        load_memory: The user's global persistent-memory preference
-            (``agent_settings.agent_context.load_memory``).  An ``AgentProfile``
-            has no ``agent_context`` field, so the preference cannot ride the
-            profile — it is stamped onto the resolved agent below, else a
-            profile-launched conversation would silently ignore the setting.
 
     Raises:
         ProfileNotFound: No stored profile has ``profile_id``.
@@ -385,15 +392,7 @@ def _resolve_agent_from_profile(
         agent = agent.model_copy(
             update={"tools": [*agent.tools, Tool(name=BROWSER_TOOL_NAME)]}
         )
-    # Persistent memory is a global user preference, not a profile field, so it
-    # is carried across the profile-resolution boundary the same way the global
-    # ``mcp_config`` is. Left untouched when off, so the resolved agent stays
-    # byte-identical for everyone who hasn't opted in.
-    if load_memory:
-        context = agent.agent_context or AgentContext()
-        agent = agent.model_copy(
-            update={"agent_context": context.model_copy(update={"load_memory": True})}
-        )
+
     launched = LaunchedAgentProfile(
         agent_profile_id=profile.id,
         revision=profile.revision,
@@ -1456,31 +1455,39 @@ async def _start_conversation(
                     f"to a different workspace"
                 )
 
-        # Profile resolution must happen before _prepare_request_workspace (which
-        # asserts request.agent is not None) and before model_dump so the resolved
-        # agent is captured in request_data.
+        # Profile resolution and the load_memory stamp must happen before
+        # _prepare_request_workspace (which asserts request.agent is not None)
+        # and before model_dump so the resolved agent is captured in request_data.
         launched_agent_profile: LaunchedAgentProfile | None = None
-        if request.agent_profile_id is not None:
-            # get_settings_store() is safe here: get_instance() initialises the
-            # singleton with the server cipher before any conversation can start.
-            from openhands.agent_server.persistence import (
-                PersistedSettings,
-                get_settings_store,
-            )
 
-            settings = get_settings_store().load() or PersistedSettings()
+        from openhands.agent_server.persistence import (
+            PersistedSettings,
+            get_settings_store,
+        )
+
+        # get_settings_store() is safe here: get_instance() initialises the
+        # singleton with the server cipher before any conversation can start.
+        settings = get_settings_store().load() or PersistedSettings()
+
+        # ``ACPAgentSettings.agent_context`` is nullable, hence the guard.
+        stored_context = settings.agent_settings.agent_context
+        load_memory = bool(stored_context and stored_context.load_memory)
+
+        if request.agent_profile_id is not None:
             mcp_config = settings.agent_settings.mcp_config
-            # ``ACPAgentSettings.agent_context`` is nullable, hence the guard.
-            stored_context = settings.agent_settings.agent_context
             resolved_agent, launched_agent_profile = await asyncio.to_thread(
                 _resolve_agent_from_profile,
                 request.agent_profile_id,
                 self.cipher,
                 mcp_config,
-                load_memory=bool(stored_context and stored_context.load_memory),
             )
             request = request.model_copy(update={"agent": resolved_agent})
 
+        if load_memory and request.agent is not None:
+            request = request.model_copy(
+                update={"agent": _with_load_memory(request.agent)}
+            )
+
         additions = request.agent_launch_additions
         suffix = (
             additions.system_message_suffix_append.strip()