diff --git a/tests/cloud/test_harness_app_contract.py b/tests/cloud/test_harness_app_contract.py index 6e3300e69..524306a6c 100644 --- a/tests/cloud/test_harness_app_contract.py +++ b/tests/cloud/test_harness_app_contract.py @@ -1224,12 +1224,21 @@ def tool(): tool_names = [getattr(tool, "__name__", "") for tool in cloned.tools] assert tool_names == ["custom_tool", "link_reader"] - def test_spawn_replaces_skills(self, monkeypatch): + def test_spawn_merges_skills_into_single_toolset(self, monkeypatch): from veadk.cloud.harness_app import utils + class FakeSkill: + def __init__(self, name): + self.name = name + class FakeSkillToolset: - def __init__(self, names): - self.names = names + def __init__(self, names=None, skills=None, code_executor=None): + self.names = names or [skill.name for skill in skills or []] + self._skills = [FakeSkill(name) for name in self.names] + self._code_executor = code_executor + + def _list_skills(self): + return self._skills old_skill_toolset = FakeSkillToolset(["old"]) @@ -1258,7 +1267,7 @@ def fake_build_skill_toolset(skill_ids, download_dir=None): ] assert old_skill_toolset not in cloned.tools assert len(skill_toolsets) == 1 - assert skill_toolsets[0].names == ["team/new-skill"] + assert skill_toolsets[0].names == ["old", "team/new-skill"] def test_spawn_clears_builtin_tools_and_skills_with_empty_overrides( self, monkeypatch diff --git a/tests/cloud/test_harness_skill_download.py b/tests/cloud/test_harness_skill_download.py index c38f8fd2e..202a3ed19 100644 --- a/tests/cloud/test_harness_skill_download.py +++ b/tests/cloud/test_harness_skill_download.py @@ -152,7 +152,7 @@ def fail_skillhub_download(*args, **kwargs): assert isinstance(toolset._code_executor, UnsafeLocalCodeExecutor) -def test_replace_skills_replaces_existing_skill_toolset(monkeypatch, tmp_path): +def test_replace_skills_merges_into_single_skill_toolset(monkeypatch, tmp_path): existing_dir = tmp_path / "existing" incoming_dir = tmp_path / "incoming" _write_adk_skill(existing_dir, name="existing") @@ -183,8 +183,15 @@ def fake_build_skill_toolset(skill_ids, download_dir=None): utils._replace_skills(agent, ["incoming"], download_dir=tmp_path) assert calls == [(["incoming"], tmp_path)] - assert agent.tools == [marker_tool, incoming_toolset] - assert [skill.name for skill in incoming_toolset._list_skills()] == ["incoming"] + assert marker_tool in agent.tools + skill_toolsets = [tool for tool in agent.tools if isinstance(tool, SkillToolset)] + assert len(skill_toolsets) == 1 + merged_toolset = skill_toolsets[0] + assert [skill.name for skill in merged_toolset._list_skills()] == [ + "existing", + "incoming", + ] + assert merged_toolset._code_executor is existing_toolset._code_executor @pytest.mark.asyncio diff --git a/veadk/cloud/harness_app/utils.py b/veadk/cloud/harness_app/utils.py index d960f4414..3a5b8a1fb 100644 --- a/veadk/cloud/harness_app/utils.py +++ b/veadk/cloud/harness_app/utils.py @@ -20,8 +20,9 @@ :class:`HarnessConfig` and builds the long-lived agent, downloading its skills from the skill hub and mounting them as an ADK skill toolset. * :func:`spawn_harness_agent` — temporary, one-off creation that clones the base - agent and applies a per-request override (configured tools/skills replace the - base harness selection). + agent and applies a per-request override (configured tools replace the base + harness selection; configured skills are merged into a single ADK skill + toolset). * :func:`spawn_harness_run_agent` — per-turn clone that also attaches dynamic registry-discovered remote A2A tools for the current user message. """ @@ -1097,12 +1098,51 @@ def _is_harness_builtin_tool(tool: Any) -> bool: def _replace_skills( agent: Agent, skill_ids: list[str], download_dir: Path | None = None ) -> None: - """Replace the harness-selected skill toolset with ``skill_ids``.""" + """Merge ``skill_ids`` into the agent's existing skills as one toolset. + ADK ``SkillToolset`` exposes fixed management tools (``list_skills``, + ``load_skill``, ``load_skill_resource``, ``run_skill_script``). Mounting two + toolsets therefore produces duplicate model tool names, so per-invoke skills + must be folded into one replacement toolset instead of appended separately. + """ + + existing_toolsets = [tool for tool in agent.tools if isinstance(tool, SkillToolset)] agent.tools = [tool for tool in agent.tools if not isinstance(tool, SkillToolset)] + if not skill_ids: + return + + existing_skills = [ + skill for toolset in existing_toolsets for skill in toolset._list_skills() + ] toolset = build_skill_toolset(skill_ids, download_dir=download_dir) - if toolset is not None: - agent.tools.append(toolset) + incoming_skills = toolset._list_skills() if toolset is not None else [] + + merged_skills = [] + seen_skill_names = set() + for skill in [*existing_skills, *incoming_skills]: + if skill.name in seen_skill_names: + continue + merged_skills.append(skill) + seen_skill_names.add(skill.name) + if not merged_skills: + return + + code_executor = next( + ( + existing_toolset._code_executor + for existing_toolset in existing_toolsets + if existing_toolset._code_executor is not None + ), + None, + ) + if code_executor is None and toolset is not None: + code_executor = toolset._code_executor + agent.tools.append( + SkillToolset( + skills=merged_skills, + code_executor=code_executor, + ) + ) def _remove_a2a_registry_tools(agent: Agent) -> None: