perf(agent_server)!: trim conversation skills by default (include_skills=false)

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

@@ -36,6 +36,7 @@
     Success,
     UpdateConversationRequest,
     UpdateSecretsRequest,
+    trim_conversation_response_skills,
 )
 from openhands.sdk import LLM, Agent, TextContent
 from openhands.sdk.conversation.state import ConversationExecutionStatus
@@ -91,9 +92,13 @@ async def search_conversations(
     """Search / List conversations"""
     assert limit > 0
     assert limit <= 100
-    return await conversation_service.search_conversations(
+    page = await conversation_service.search_conversations(
         page_id, limit, status, sort_order
     )
+    # Drop ``agent.agent_context.skills`` from each item before
+    # serialization — see ``trim_conversation_response_skills``.
+    page.items = [trim_conversation_response_skills(item) for item in page.items]
+    return page
 
 
 @conversation_router.get("/count")
@@ -120,7 +125,7 @@ async def get_conversation(
     conversation = await conversation_service.get_conversation(conversation_id)
     if conversation is None:
         raise HTTPException(status.HTTP_404_NOT_FOUND)
-    return conversation
+    return trim_conversation_response_skills(conversation)
 
 
 @conversation_router.get(
@@ -153,7 +158,10 @@ async def batch_get_conversations(
     any missing item"""
     assert len(ids) < 100
     conversations = await conversation_service.batch_get_conversations(ids)
-    return conversations
+    return [
+        trim_conversation_response_skills(c) if c is not None else None
+        for c in conversations
+    ]
 
 
 # Write Methods
@@ -170,7 +178,7 @@ async def start_conversation(
     """Start a conversation in the local environment."""
     info, is_new = await conversation_service.start_conversation(request)
     response.status_code = status.HTTP_201_CREATED if is_new else status.HTTP_200_OK
-    return info
+    return trim_conversation_response_skills(info)
 
 
 @conversation_router.post(
@@ -432,4 +440,4 @@ async def fork_conversation(
             status.HTTP_404_NOT_FOUND,
             detail="Source conversation not found",
         )
-    return info
+    return trim_conversation_response_skills(info)

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

@@ -19,6 +19,7 @@
     ConversationSortOrder,
     SendMessageRequest,
     StartACPConversationRequest,
+    trim_conversation_response_skills,
 )
 from openhands.sdk import LLM, Agent, TextContent
 from openhands.sdk.agent.acp_agent import ACPAgent
@@ -85,9 +86,11 @@ async def search_acp_conversations(
     """
     assert limit > 0
     assert limit <= 100
-    return await conversation_service.search_acp_conversations(
+    page = await conversation_service.search_acp_conversations(
         page_id, limit, status, sort_order
     )
+    page.items = [trim_conversation_response_skills(item) for item in page.items]
+    return page
 
 
 @conversation_router_acp.get("/count", deprecated=True)
@@ -123,7 +126,7 @@ async def get_acp_conversation(
     conversation = await conversation_service.get_acp_conversation(conversation_id)
     if conversation is None:
         raise HTTPException(status.HTTP_404_NOT_FOUND)
-    return conversation
+    return trim_conversation_response_skills(conversation)
 
 
 @conversation_router_acp.get("", deprecated=True)
@@ -137,7 +140,11 @@ async def batch_get_acp_conversations(
     Use ``/api/conversations`` instead.
     """
     assert len(ids) < 100
-    return await conversation_service.batch_get_acp_conversations(ids)
+    conversations = await conversation_service.batch_get_acp_conversations(ids)
+    return [
+        trim_conversation_response_skills(c) if c is not None else None
+        for c in conversations
+    ]
 
 
 @conversation_router_acp.post("", deprecated=True)
@@ -157,4 +164,4 @@ async def start_acp_conversation(
     """
     info, is_new = await conversation_service.start_acp_conversation(request)
     response.status_code = status.HTTP_201_CREATED if is_new else status.HTTP_200_OK
-    return info
+    return trim_conversation_response_skills(info)

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

@@ -200,6 +200,45 @@ class ConversationPage(BaseModel):
     next_page_id: str | None = None
 
 
+def trim_conversation_response_skills(info: ConversationInfo) -> ConversationInfo:
+    """Return ``info`` with ``agent.agent_context.skills`` set to ``[]``.
+
+    Applied at the four HTTP read routes that emit ``ConversationInfo``
+    (search, get, batch-get, start). The persisted ``ConversationState``
+    on disk and the in-memory copy held by the agent's runtime are
+    untouched — only the bytes leaving over HTTP shrink.
+
+    Why drop the skills entirely on the wire: when an ``AgentContext``
+    is constructed with ``load_user_skills=True`` / ``load_public_skills=True``,
+    its model_validator resolves the entire skill catalog (~40 entries
+    in stock setups) and persists them inline. Every conversation
+    fetch therefore carried ~260 KB of skill content that no API
+    consumer actually reads — the skill bodies are only consumed
+    server-side at prompt-render time, never client-side.
+
+    Dropping ``skills`` from the API response (rather than from the
+    model itself) keeps the model semantics simple: the wire
+    representation differs from the in-memory representation in
+    exactly one place, here, instead of via a serializer / validator
+    pair that has to coordinate across persistence, ``model_copy``,
+    ``round_trip``, equality checks, snapshot drift detection, etc.
+
+    A ``model_copy`` chain is enough because ``BaseModel.model_copy``
+    is shallow on default — we replace the leaf ``skills`` list with
+    an empty list without touching any other field. The returned
+    object is a fresh ``ConversationInfo`` instance; callers that
+    hold the input reference observe no mutation.
+    """
+    agent_ctx = getattr(info.agent, "agent_context", None)
+    if agent_ctx is None or not agent_ctx.skills:
+        return info
+    trimmed_agent_context = agent_ctx.model_copy(update={"skills": []})
+    trimmed_agent = info.agent.model_copy(
+        update={"agent_context": trimmed_agent_context}
+    )
+    return info.model_copy(update={"agent": trimmed_agent})
+
+
 # Deprecated compatibility aliases for the old ACP-specific response names.
 # Keep runtime assignment aliases so existing imports still resolve to the
 # canonical Pydantic models; PEP 695 ``type`` aliases would not preserve that.

tests/agent_server/test_conversation_router_skill_trim.py

@@ -0,0 +1,214 @@
+"""Tests for the route-level ``agent.agent_context.skills`` trim.
+
+The four read endpoints (``GET /search``, ``GET /{id}``, ``GET ""``,
+``POST ""``) on the conversation router strip ``agent.agent_context.skills``
+from the response payload. The persisted ``ConversationState`` and the
+in-memory copy held by the agent's runtime are unaffected — only the
+bytes leaving over HTTP shrink.
+
+See the SDK PR description for why this lives at the route boundary
+rather than inside ``AgentContext`` itself.
+"""
+
+from __future__ import annotations
+
+from unittest.mock import AsyncMock
+from uuid import uuid4
+
+import pytest
+from fastapi import FastAPI
+from fastapi.testclient import TestClient
+from pydantic import SecretStr
+
+from openhands.agent_server.config import Config
+from openhands.agent_server.conversation_router import conversation_router
+from openhands.agent_server.conversation_service import ConversationService
+from openhands.agent_server.dependencies import get_conversation_service
+from openhands.agent_server.models import (
+    ConversationInfo,
+    ConversationPage,
+    trim_conversation_response_skills,
+)
+from openhands.agent_server.utils import utc_now
+from openhands.sdk import LLM, Agent
+from openhands.sdk.context import AgentContext
+from openhands.sdk.conversation.state import ConversationExecutionStatus
+from openhands.sdk.skills import Skill
+from openhands.sdk.workspace import LocalWorkspace
+
+
+def _make_skill(name: str, content: str = "skill body bytes") -> Skill:
+    return Skill(name=name, content=content, source=f"/fake/{name}.md")
+
+
+def _make_conversation_with_skills(skills: list[Skill]) -> ConversationInfo:
+    """Build a ``ConversationInfo`` whose agent carries ``skills``.
+
+    The full ``AgentContext`` field set is otherwise empty so the
+    trimmed payload reflects only the skills delta.
+    """
+    now = utc_now()
+    return ConversationInfo(
+        id=uuid4(),
+        agent=Agent(
+            llm=LLM(model="gpt-4o", api_key=SecretStr("k"), usage_id="test-llm"),
+            tools=[],
+            agent_context=AgentContext(skills=skills),
+        ),
+        workspace=LocalWorkspace(working_dir="/tmp/test"),
+        execution_status=ConversationExecutionStatus.IDLE,
+        title="Test",
+        created_at=now,
+        updated_at=now,
+    )
+
+
+class TestTrimHelper:
+    """Unit tests for the pure-function helper."""
+
+    def test_strips_skills_when_present(self):
+        info = _make_conversation_with_skills(
+            [_make_skill("a"), _make_skill("b"), _make_skill("c")]
+        )
+        trimmed = trim_conversation_response_skills(info)
+        assert trimmed.agent.agent_context is not None
+        assert trimmed.agent.agent_context.skills == []
+
+    def test_returns_same_instance_when_nothing_to_strip(self):
+        # Empty skill list → identity return (no needless model_copy).
+        info = _make_conversation_with_skills([])
+        trimmed = trim_conversation_response_skills(info)
+        assert trimmed is info
+
+    def test_does_not_touch_other_agent_context_fields(self):
+        info = _make_conversation_with_skills([_make_skill("a")])
+        # Mutate a non-skill field so we can assert it survives.
+        assert info.agent.agent_context is not None
+        info = info.model_copy(
+            update={
+                "agent": info.agent.model_copy(
+                    update={
+                        "agent_context": info.agent.agent_context.model_copy(
+                            update={"system_message_suffix": "carry me through"}
+                        )
+                    }
+                )
+            }
+        )
+        trimmed = trim_conversation_response_skills(info)
+        assert trimmed.agent.agent_context is not None
+        assert trimmed.agent.agent_context.skills == []
+        assert trimmed.agent.agent_context.system_message_suffix == "carry me through"
+
+    def test_does_not_mutate_input(self):
+        info = _make_conversation_with_skills([_make_skill("a"), _make_skill("b")])
+        trim_conversation_response_skills(info)
+        # Caller's reference still sees the full skills — model_copy
+        # gave us a fresh instance, the input is untouched.
+        assert info.agent.agent_context is not None
+        assert {s.name for s in info.agent.agent_context.skills} == {"a", "b"}
+
+    def test_agent_without_agent_context_passes_through(self):
+        now = utc_now()
+        info = ConversationInfo(
+            id=uuid4(),
+            agent=Agent(
+                llm=LLM(model="gpt-4o", api_key=SecretStr("k"), usage_id="t"),
+                tools=[],
+            ),
+            workspace=LocalWorkspace(working_dir="/tmp/test"),
+            execution_status=ConversationExecutionStatus.IDLE,
+            title="Test",
+            created_at=now,
+            updated_at=now,
+        )
+        # No agent_context at all → helper is a no-op.
+        assert trim_conversation_response_skills(info) is info
+
+
+class TestRouteIntegration:
+    """Integration tests through the FastAPI router — proves the trim
+    actually fires at every read endpoint."""
+
+    @pytest.fixture
+    def heavy_conversation(self):
+        # 5 skills with non-trivial content — enough that the trim
+        # is visible in the serialized JSON byte count.
+        return _make_conversation_with_skills(
+            [_make_skill(f"skill-{i}", "x" * 500) for i in range(5)]
+        )
+
+    @pytest.fixture
+    def client(self, heavy_conversation):
+        service = AsyncMock(spec=ConversationService)
+        service.get_conversation.return_value = heavy_conversation
+        service.batch_get_conversations.return_value = [heavy_conversation]
+        service.search_conversations.return_value = ConversationPage(
+            items=[heavy_conversation], next_page_id=None
+        )
+
+        app = FastAPI()
+        app.include_router(conversation_router, prefix="/api")
+        app.state.config = Config(
+            static_files_path=None, session_api_keys=[], secret_key=None
+        )
+        app.dependency_overrides[get_conversation_service] = lambda: service
+        return TestClient(app), heavy_conversation
+
+    def test_get_conversation_trims_skills(self, client):
+        c, heavy = client
+        response = c.get(f"/api/conversations/{heavy.id}")
+        assert response.status_code == 200
+        body = response.json()
+        assert body["agent"]["agent_context"]["skills"] == []
+
+    def test_batch_get_conversations_trims_skills(self, client):
+        c, heavy = client
+        response = c.get(f"/api/conversations?ids={heavy.id}")
+        assert response.status_code == 200
+        body = response.json()
+        assert body[0]["agent"]["agent_context"]["skills"] == []
+
+    def test_batch_get_handles_null_items(self):
+        """Missing items return ``None`` and the trim doesn't crash on them."""
+        service = AsyncMock(spec=ConversationService)
+        service.batch_get_conversations.return_value = [None]
+        app = FastAPI()
+        app.include_router(conversation_router, prefix="/api")
+        app.state.config = Config(
+            static_files_path=None, session_api_keys=[], secret_key=None
+        )
+        app.dependency_overrides[get_conversation_service] = lambda: service
+        c = TestClient(app)
+        response = c.get(f"/api/conversations?ids={uuid4()}")
+        assert response.status_code == 200
+        assert response.json() == [None]
+
+    def test_search_conversations_trims_skills(self, client):
+        c, _heavy = client
+        response = c.get("/api/conversations/search")
+        assert response.status_code == 200
+        body = response.json()
+        assert body["items"][0]["agent"]["agent_context"]["skills"] == []
+
+    def test_response_size_drops_meaningfully(self, client):
+        """Compare trimmed (HTTP) vs untrimmed (model_dump_json) sizes.
+
+        The conversation has 5 skills × 500 chars of content = ~2500
+        bytes of skill bodies. The trimmed HTTP response should be at
+        least that much smaller than serializing the same conversation
+        with skills intact.
+        """
+        c, heavy = client
+        response = c.get("/api/conversations/search")
+        trimmed_bytes = len(response.content)
+        untrimmed_bytes = len(
+            ConversationPage(items=[heavy], next_page_id=None).model_dump_json()
+        )
+        # 5 × 500 chars of "x" skill content + per-skill metadata
+        # overhead. Conservatively require at least 1500 bytes shaved.
+        assert untrimmed_bytes - trimmed_bytes > 1500, (
+            f"trim should drop ~2500 B of skill content; got "
+            f"{untrimmed_bytes - trimmed_bytes} B saved "
+            f"({untrimmed_bytes} → {trimmed_bytes})"
+        )