diff --git a/openhands-sdk/openhands/sdk/context/agent_context.py b/openhands-sdk/openhands/sdk/context/agent_context.py index 17d9508427..640415e7e2 100644 --- a/openhands-sdk/openhands/sdk/context/agent_context.py +++ b/openhands-sdk/openhands/sdk/context/agent_context.py @@ -23,6 +23,7 @@ Skill, SkillKnowledge, load_available_skills, + merge_skills_by_name, to_prompt, ) from openhands.sdk.skills.skill import DEFAULT_MARKETPLACE_PATH @@ -96,6 +97,22 @@ class AgentContext(BaseModel): ), json_schema_extra={"acp_compatible": True}, ) + load_project_skills: bool = Field( + default=False, + description=( + "Whether to automatically load project skills from the conversation " + "workspace (e.g. .openhands/skills/, AGENTS.md). Unlike " + "load_user_skills / load_public_skills, this flag is not resolved by " + "AgentContext itself (the workspace path is unknown at validation " + "time); LocalConversation resolves it lazily on the first " + "send_message() / run(), when the workspace is known. Also unlike " + "load_user_skills / load_public_skills (which yield to explicit " + "skills on a name conflict), resolved project skills are " + "authoritative: a project skill overrides a same-named skill already " + "present in `skills`." + ), + json_schema_extra={"acp_compatible": True}, + ) secrets: Mapping[str, SecretValue] | None = Field( default=None, description=( @@ -160,15 +177,14 @@ def _load_auto_skills(self): marketplace_path=self.marketplace_path, ) - existing_names = {skill.name for skill in self.skills} - for name, skill in auto_skills.items(): - if name not in existing_names: - self.skills.append(skill) - else: + # Explicit skills are authoritative; auto-loaded skills only fill gaps. + explicit_names = {skill.name for skill in self.skills} + for name in auto_skills: + if name in explicit_names: logger.debug( f"Skipping auto-loaded skill '{name}' (already in explicit skills)" ) - + self.skills = merge_skills_by_name(self.skills, auto_skills.values()) return self def get_secret_infos(self) -> list[dict[str, str | None]]: diff --git a/openhands-sdk/openhands/sdk/conversation/impl/local_conversation.py b/openhands-sdk/openhands/sdk/conversation/impl/local_conversation.py index a6bb1ec60e..9596a4997b 100644 --- a/openhands-sdk/openhands/sdk/conversation/impl/local_conversation.py +++ b/openhands-sdk/openhands/sdk/conversation/impl/local_conversation.py @@ -58,6 +58,7 @@ from openhands.sdk.security.confirmation_policy import ( ConfirmationPolicyBase, ) +from openhands.sdk.skills import load_available_skills, merge_skills_by_name from openhands.sdk.skills.utils import expand_mcp_variables from openhands.sdk.subagent import ( AgentDefinition, @@ -504,6 +505,38 @@ def _ensure_plugins_loaded(self) -> None: logger.info(f"Loaded {len(self._plugin_specs)} plugin(s) via Conversation") + # Resolve project skills from the workspace. AgentContext can't do this + # itself (the workspace path is unknown at validation time), so it is done + # here, where the path is known. Project skills take precedence over + # same-named skills already on the context. + project_skills_loaded = False + if merged_context is not None and merged_context.load_project_skills: + # Best-effort: a failure to load project skills must not prevent the + # conversation from starting. (load_available_skills already guards + # the project source internally; this is belt-and-suspenders.) + try: + project_skills = load_available_skills( + work_dir=self.workspace.working_dir, + include_user=False, + include_project=True, + include_public=False, + ) + except Exception: + logger.warning( + "Failed to load project skills; continuing without them", + exc_info=True, + ) + project_skills = {} + if project_skills: + # Project skills are authoritative over same-named context skills. + merged_skills = merge_skills_by_name( + project_skills.values(), merged_context.skills + ) + merged_context = merged_context.model_copy( + update={"skills": merged_skills} + ) + project_skills_loaded = True + # Expand MCP config variables with per-conversation secrets # This handles ${VAR} and ${VAR:-default} placeholders: # - Variables referencing secrets injected via API are expanded to secret values @@ -521,9 +554,9 @@ def _ensure_plugins_loaded(self) -> None: ) logger.debug("Expanded MCP config variables") - # Update agent with merged content only if we have plugins or MCP config - # Skip update when nothing changed to avoid unnecessary agent state mutations - if self._plugin_specs or has_mcp_config: + # Update agent with merged content only if something changed. + # Skip update otherwise to avoid unnecessary agent state mutations. + if self._plugin_specs or has_mcp_config or project_skills_loaded: self.agent = self.agent.model_copy( update={ "agent_context": merged_context, diff --git a/openhands-sdk/openhands/sdk/skills/__init__.py b/openhands-sdk/openhands/sdk/skills/__init__.py index 62a47f09dc..124043ef12 100644 --- a/openhands-sdk/openhands/sdk/skills/__init__.py +++ b/openhands-sdk/openhands/sdk/skills/__init__.py @@ -10,6 +10,7 @@ - `load_user_skills` - Load skills from ~/.openhands/skills/ - `load_public_skills` - Load skills from the public OpenHands extensions repo - `load_available_skills` - Load and merge skills from multiple sources +- `merge_skills_by_name` - Merge two skill collections by name (primary wins) **Triggers:** - `BaseTrigger`, `KeywordTrigger`, `TaskTrigger` - Skill activation triggers @@ -63,6 +64,7 @@ load_public_skills, load_skills_from_dir, load_user_skills, + merge_skills_by_name, to_prompt, ) @@ -117,6 +119,7 @@ "load_user_skills", "load_public_skills", "load_available_skills", + "merge_skills_by_name", "to_prompt", # Triggers "BaseTrigger", diff --git a/openhands-sdk/openhands/sdk/skills/skill.py b/openhands-sdk/openhands/sdk/skills/skill.py index c47e4eee1a..6b1d6afa6f 100644 --- a/openhands-sdk/openhands/sdk/skills/skill.py +++ b/openhands-sdk/openhands/sdk/skills/skill.py @@ -4,6 +4,7 @@ import re import threading import time +from collections.abc import Iterable from pathlib import Path from typing import Annotated, ClassVar, Literal, Union from xml.sax.saxutils import escape as xml_escape @@ -1203,6 +1204,24 @@ def load_available_skills( return available +def merge_skills_by_name( + primary: Iterable[Skill], secondary: Iterable[Skill] +) -> list[Skill]: + """Merge two skill collections by name. + + ``primary`` skills are authoritative: they take precedence on name conflicts + and keep their order. Each ``secondary`` skill is appended only when its name + is not already provided by ``primary``. + """ + merged = list(primary) + seen = {skill.name for skill in merged} + for skill in secondary: + if skill.name not in seen: + seen.add(skill.name) + merged.append(skill) + return merged + + def to_prompt(skills: list[Skill], max_description_length: int = 1024) -> str: """Generate XML prompt block for available skills. diff --git a/tests/sdk/conversation/test_repo_root_project_skills.py b/tests/sdk/conversation/test_repo_root_project_skills.py index 052b3f247d..a0e38b2a37 100644 --- a/tests/sdk/conversation/test_repo_root_project_skills.py +++ b/tests/sdk/conversation/test_repo_root_project_skills.py @@ -1,16 +1,29 @@ from __future__ import annotations from pathlib import Path +from unittest.mock import patch from openhands.sdk.agent import Agent from openhands.sdk.context.agent_context import AgentContext from openhands.sdk.conversation.impl.local_conversation import LocalConversation from openhands.sdk.event import SystemPromptEvent from openhands.sdk.llm import Message, TextContent -from openhands.sdk.skills import load_project_skills +from openhands.sdk.skills import Skill, load_project_skills from openhands.sdk.testing import TestLLM +def _agent(agent_context: AgentContext) -> Agent: + return Agent( + llm=TestLLM.from_messages( + [Message(role="assistant", content=[TextContent(text="ok")])], + model="test-model", + ), + tools=[], + include_default_tools=[], + agent_context=agent_context, + ) + + def test_system_prompt_includes_repo_root_agents_md_when_workdir_is_subdir( tmp_path: Path, ): @@ -69,3 +82,124 @@ def test_system_prompt_includes_repo_root_agents_md_when_workdir_is_subdir( assert "SENTINEL_ROOT_123" in system_prompt_event.dynamic_context.text conversation.close() + + +def test_load_project_skills_flag_injects_skills_in_standalone_sdk(tmp_path: Path): + """``AgentContext(load_project_skills=True)`` works without agent-server. + + LocalConversation resolves project skills from the workspace at startup, + so the flag behaves consistently for standalone SDK usage (agent-canvas#574). + """ + (tmp_path / "AGENTS.md").write_text("# Guidelines\n\nSENTINEL_FLAG_456\n") + + agent = _agent( + AgentContext( + load_project_skills=True, + current_datetime="2026-01-01T00:00:00Z", + ) + ) + conversation = LocalConversation( + agent=agent, + workspace=tmp_path, + persistence_dir=tmp_path / "conversation", + delete_on_close=True, + ) + conversation.send_message("hi") + + # Skills are merged into the live agent context... + assert conversation.agent.agent_context is not None + assert "agents" in {s.name for s in conversation.agent.agent_context.skills} + # ...and rendered into the system prompt. + system_prompt_event = next( + e for e in conversation.state.events if isinstance(e, SystemPromptEvent) + ) + assert system_prompt_event.dynamic_context is not None + assert "SENTINEL_FLAG_456" in system_prompt_event.dynamic_context.text + + conversation.close() + + +def test_load_project_skills_flag_off_does_not_inject(tmp_path: Path): + """With the flag unset (default), project skills are not loaded.""" + (tmp_path / "AGENTS.md").write_text("# Guidelines\n\nSENTINEL_OFF_789\n") + + agent = _agent(AgentContext(current_datetime="2026-01-01T00:00:00Z")) + conversation = LocalConversation( + agent=agent, + workspace=tmp_path, + persistence_dir=tmp_path / "conversation", + delete_on_close=True, + ) + conversation.send_message("hi") + + assert conversation.agent.agent_context is not None + assert conversation.agent.agent_context.skills == [] + system_prompt_event = next( + e for e in conversation.state.events if isinstance(e, SystemPromptEvent) + ) + dynamic = system_prompt_event.dynamic_context + assert dynamic is None or "SENTINEL_OFF_789" not in dynamic.text + + conversation.close() + + +def test_load_project_skills_flag_merges_with_project_precedence(tmp_path: Path): + """Project skills override same-named context skills; others are preserved.""" + (tmp_path / "AGENTS.md").write_text("# Guidelines\n\nSENTINEL_NEW\n") + + agent = _agent( + AgentContext( + skills=[ + Skill(name="keep", content="keep me"), + Skill(name="agents", content="OLD_CONTENT"), + ], + load_project_skills=True, + current_datetime="2026-01-01T00:00:00Z", + ) + ) + conversation = LocalConversation( + agent=agent, + workspace=tmp_path, + persistence_dir=tmp_path / "conversation", + delete_on_close=True, + ) + conversation.send_message("hi") + + assert conversation.agent.agent_context is not None + skills = {s.name: s for s in conversation.agent.agent_context.skills} + assert skills["keep"].content == "keep me" + assert "SENTINEL_NEW" in skills["agents"].content + assert "OLD_CONTENT" not in skills["agents"].content + + conversation.close() + + +def test_load_project_skills_failure_does_not_block_conversation(tmp_path: Path): + """Project-skill loading is best-effort: a load error must not break startup.""" + (tmp_path / "AGENTS.md").write_text("# Guidelines\n\nSENTINEL\n") + + agent = _agent( + AgentContext( + skills=[Skill(name="keep", content="keep me")], + load_project_skills=True, + current_datetime="2026-01-01T00:00:00Z", + ) + ) + conversation = LocalConversation( + agent=agent, + workspace=tmp_path, + persistence_dir=tmp_path / "conversation", + delete_on_close=True, + ) + + with patch( + "openhands.sdk.conversation.impl.local_conversation.load_available_skills", + side_effect=PermissionError("workspace unreadable"), + ): + conversation.send_message("hi") # must not raise + + # Conversation started; pre-existing skills are untouched. + assert conversation.agent.agent_context is not None + assert {s.name for s in conversation.agent.agent_context.skills} == {"keep"} + + conversation.close()