perf(agent_context): drop auto-loaded skills from serialized output (-263 KB/conversation, -48% cold open)

openhands-sdk/openhands/sdk/context/agent_context.py

@@ -8,6 +8,7 @@
 from pydantic import (
     BaseModel,
     Field,
+    PrivateAttr,
     SecretStr,
     field_serializer,
     field_validator,
@@ -118,6 +119,93 @@ class AgentContext(BaseModel):
         json_schema_extra={"acp_compatible": True},
     )
 
+    # Snapshot of the skills that ``_load_auto_skills`` added on top of
+    # the caller-supplied list. The serializer drops only the ones still
+    # equal to this snapshot — if a downstream consumer replaces an
+    # auto-loaded skill via ``model_copy(update={'skills': merged})``
+    # (OpenHands' ``_create_agent_with_skills`` does this when it merges
+    # in sandbox / repo skills with overlapping names), the replacement
+    # has different field values and survives serialization. The same
+    # auto-load will re-run on the receiving end and rebuild the
+    # auto-loaded subset that wasn't replaced. For a stock configuration
+    # that turns both flags on (~40 skills bundled under
+    # ``~/.openhands/skills``) the resolved list is ~260 KB per
+    # ``AgentContext`` — every ``GET`` on a stored conversation carried
+    # that. See software-agent-sdk#3301.
+    #
+    # Values are deep-copied so an in-place caller mutation
+    # (``ctx.skills[0].content = "custom"``) doesn't also mutate the
+    # snapshot — without that, the equality check would still succeed
+    # and the customised skill would silently disappear from the wire.
+    _auto_loaded_skills: dict[str, Skill] = PrivateAttr(default_factory=dict)
+    # The auto-load config that produced ``_auto_loaded_skills``.
+    # Tracked so the serializer can no-op the trim when the config
+    # changed after validation — e.g. ``model_copy(update={
+    # "load_public_skills": False})``. Without this, a copy with the
+    # flag flipped off would drop the auto-loaded skills on the wire
+    # AND fail to re-load them on the next ``model_validate`` (the
+    # flag is now off), losing them entirely. ``None`` when
+    # ``_load_auto_skills`` has not run yet.
+    _auto_load_config: tuple[bool, bool, str | None] | None = PrivateAttr(default=None)
+
+    @field_serializer("skills", when_used="always", mode="wrap")
+    def _serialize_skills(self, value: list[Skill], handler, info) -> Any:
+        """Drop unmodified auto-loaded skills from the serialized output.
+
+        The runtime keeps the full resolved list on ``self.skills`` so
+        prompt rendering and downstream consumers behave exactly as
+        today. Only the wire payload changes: callers re-loading the
+        model will trigger ``_load_auto_skills`` again, which rebuilds
+        the auto-loaded subset deterministically from the same
+        ``load_user_skills`` / ``load_public_skills`` /
+        ``marketplace_path`` configuration.
+
+        Equality (not just name match) is required because consumers
+        like OpenHands' ``_create_agent_with_skills`` replace
+        auto-loaded skills in-place with their own version under the
+        same name. A name-only filter would silently drop those
+        replacements and the receiver would auto-reload the stock
+        version on the next deserialization.
+
+        Opt-out via ``context={"preserve_full_skills": True}``: paths
+        that need a stable snapshot of the resolved skill catalog (the
+        most important one being ``ConversationState._save_base_state``
+        — persistence of the conversation to disk) pass this flag so
+        the serializer is a no-op. Without it, a paused conversation
+        resumed after the ``~/.openhands/skills`` directory or the
+        public marketplace updated would silently pick up the *new*
+        skill content via ``_load_auto_skills``. The API-response path
+        skips the flag and gets the byte-size win.
+
+        Config-drift safety: if the auto-load config
+        (``load_user_skills`` / ``load_public_skills`` /
+        ``marketplace_path``) changed since the snapshot was taken —
+        typically via ``model_copy(update=...)`` flipping one of those
+        flags — the trim is skipped. Otherwise the receiver would
+        either re-load a *different* skill catalog (changed
+        ``marketplace_path``) or fail to re-load at all (flag turned
+        off), losing the auto-loaded skills entirely.
+
+        ``mode="wrap"`` + ``handler`` delegation preserves caller
+        options like ``exclude_none``, ``exclude_defaults``, and
+        nested ``include`` / ``exclude`` for the surviving skills —
+        manual ``s.model_dump(...)`` would have ignored them.
+        """
+        if info.context and info.context.get("preserve_full_skills"):
+            return handler(value)
+        # Auto-load config drifted (or never ran) → can't trust the
+        # snapshot to round-trip. Serialize everything as explicit.
+        current_config = (
+            self.load_user_skills,
+            self.load_public_skills,
+            self.marketplace_path,
+        )
+        if self._auto_load_config != current_config:
+            return handler(value)
+        auto = self._auto_loaded_skills
+        kept = [s for s in value if auto.get(s.name) != s]
+        return handler(kept)
+
     @field_serializer("secrets", when_used="always")
     def _serialize_secrets(
         self, value: Mapping[str, SecretValue] | None, info
@@ -148,7 +236,25 @@ def _validate_skills(cls, v: list[Skill], _info):
 
     @model_validator(mode="after")
     def _load_auto_skills(self):
-        """Load user and/or public skills if enabled."""
+        """Load user and/or public skills if enabled.
+
+        Names of skills added here are tracked in
+        ``_auto_loaded_skill_names`` so the serializer can drop them
+        from the wire payload (this validator re-runs on every model
+        load, so the same skills repopulate without needing to be
+        persisted).
+
+        Migration: stored conversations created before the serializer
+        change carry the resolved auto-loaded skill list inlined on
+        ``skills``. When such a conversation is loaded back, the names
+        match ``existing_names`` and the new-append branch is skipped
+        — but we still mark them as auto-loaded if the persisted skill
+        equals what the loader would produce now. Without this, every
+        old conversation would keep the bloated payload until rewritten.
+        A persisted skill that no longer matches the loader's current
+        output (user edited the file, marketplace updated, etc.) stays
+        treated as explicit so the on-disk content wins.
+        """
         if not self.load_user_skills and not self.load_public_skills:
             return self
 
@@ -160,10 +266,31 @@ def _load_auto_skills(self):
             marketplace_path=self.marketplace_path,
         )
 
-        existing_names = {skill.name for skill in self.skills}
+        # Record the config that produced this snapshot so the
+        # serializer can detect drift (e.g. ``model_copy(update={
+        # "load_public_skills": False})``) and degrade to full
+        # serialization.
+        self._auto_load_config = (
+            self.load_user_skills,
+            self.load_public_skills,
+            self.marketplace_path,
+        )
+
+        existing_by_name = {skill.name: skill for skill in self.skills}
         for name, skill in auto_skills.items():
-            if name not in existing_names:
+            existing = existing_by_name.get(name)
+            if existing is None:
                 self.skills.append(skill)
+                # Deep-copy so an in-place caller mutation
+                # (``ctx.skills[0].content = "custom"``) doesn't also
+                # mutate the snapshot — without it the equality check
+                # would still succeed and the customised skill would
+                # silently disappear from the wire.
+                self._auto_loaded_skills[name] = skill.model_copy(deep=True)
+            elif existing == skill:
+                # Migration path for conversations stored before the
+                # serializer change — see the docstring.
+                self._auto_loaded_skills[name] = skill.model_copy(deep=True)
             else:
                 logger.debug(
                     f"Skipping auto-loaded skill '{name}' (already in explicit skills)"

openhands-sdk/openhands/sdk/conversation/state.py

@@ -255,8 +255,18 @@ def _save_base_state(self, fs: FileStore) -> None:
 
         If a cipher is configured, secrets will be encrypted. Otherwise, they
         will be redacted (serialized as '**********').
+
+        ``preserve_full_skills`` is set so the persisted snapshot
+        includes auto-loaded skills inline — without this the conversation
+        would silently pick up *new* skill content from
+        ``~/.openhands/skills`` / the public marketplace on resume, since
+        ``AgentContext._load_auto_skills`` re-runs on deserialization.
+        Persistence needs the freeze-at-create-time snapshot; only the
+        API-response path opts into the trim. See software-agent-sdk#3301.
         """
-        context = {"cipher": self._cipher} if self._cipher else None
+        context: dict[str, Any] = {"preserve_full_skills": True}
+        if self._cipher:
+            context["cipher"] = self._cipher
         # Warn if secrets exist but no cipher is configured
         if not self._cipher and self.secret_registry.secret_sources:
             logger.warning(