Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 12 additions & 3 deletions src/google/adk/tools/skill_toolset.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@
" conversation history for you to analyze."
)


def _build_skill_system_instruction(prefix: str | None = None) -> str:
p = f"{prefix}_" if prefix else ""

Expand All @@ -75,7 +76,7 @@ def _build_skill_system_instruction(prefix: str | None = None) -> str:
"bash.\n\n"
"This is very important:\n\n"
f"1. If a skill seems relevant to the current user query, you MUST use "
f"the `{p}load_skill` tool with `skill_name=\"<SKILL_NAME>\"` to read "
f'the `{p}load_skill` tool with `skill_name="<SKILL_NAME>"` to read '
"its full instructions before proceeding.\n"
"2. Once you have read the instructions, follow them exactly as "
"documented before replying to the user. For example, If the "
Expand All @@ -85,7 +86,8 @@ def _build_skill_system_instruction(prefix: str | None = None) -> str:
"skill's directory (e.g., `references/*`, `assets/*`, `scripts/*`). "
"Do NOT use other tools to access these files.\n"
f"4. Use `{p}run_skill_script` to run scripts from a skill's `scripts/` "
f"directory. Use `{p}load_skill_resource` to view script content first if "
f"directory. Use `{p}load_skill_resource` to view script content"
" first if "
"needed.\n"
)

Expand Down Expand Up @@ -666,6 +668,10 @@ def _build_wrapper_code(

code_lines.extend([
f" sys.argv = {argv_list!r}",
(
" sys.path.insert(0,"
f" os.path.dirname(os.path.abspath({file_path!r})))"
),
" try:",
f" runpy.run_path({file_path!r}, run_name='__main__')",
" except SystemExit as e:",
Expand Down Expand Up @@ -1085,7 +1091,9 @@ async def process_llm_request(
self, *, tool_context: ToolContext, llm_request: LlmRequest
) -> None:
"""Processes the outgoing LLM request to include available skills."""
instructions = [_build_skill_system_instruction(prefix=self.tool_name_prefix)]
instructions = [
_build_skill_system_instruction(prefix=self.tool_name_prefix)
]

has_list_skills = any(isinstance(t, ListSkillsTool) for t in self._tools)

Expand Down Expand Up @@ -1114,4 +1122,5 @@ async def close(self) -> None:
self._fetched_skill_cache.clear()
await super().close()


DEFAULT_SKILL_SYSTEM_INSTRUCTION = _build_skill_system_instruction()
45 changes: 41 additions & 4 deletions tests/unittests/tools/test_skill_toolset.py
Original file line number Diff line number Diff line change
Expand Up @@ -636,6 +636,10 @@ async def test_execute_script_python_success(mock_skill1):
assert "_materialize_and_run()" in code_input.code
assert "import runpy" in code_input.code
assert "sys.argv = ['scripts/run.py']" in code_input.code
assert (
"sys.path.insert(0, os.path.dirname(os.path.abspath('scripts/run.py')))"
in code_input.code
)
assert (
"runpy.run_path('scripts/run.py', run_name='__main__')" in code_input.code
)
Expand Down Expand Up @@ -1025,6 +1029,11 @@ def get_script_extended(name):

def _make_skill_with_script(skill_name, script_name, script):
"""Creates a minimal mock Skill with a single script."""
return _make_skill_with_scripts(skill_name, {script_name: script})


def _make_skill_with_scripts(skill_name, scripts):
"""Creates a minimal mock Skill with scripts."""
skill = mock.create_autospec(models.Skill, instance=True)
skill.name = skill_name
skill.description = f"Test skill {skill_name}"
Expand All @@ -1045,16 +1054,14 @@ def _make_skill_with_script(skill_name, script_name, script):
)

def get_script(name):
if name == script_name:
return script
return None
return scripts.get(name)

skill.resources.get_script.side_effect = get_script
skill.resources.get_reference.return_value = None
skill.resources.get_asset.return_value = None
skill.resources.list_references.return_value = []
skill.resources.list_assets.return_value = []
skill.resources.list_scripts.return_value = [script_name]
skill.resources.list_scripts.return_value = list(scripts)
return skill


Expand Down Expand Up @@ -1089,6 +1096,36 @@ async def test_integration_python_stdout():
assert result["stderr"] == ""


@pytest.mark.asyncio
async def test_integration_python_imports_sibling_script_module():
"""Real executor: Python scripts can import helpers from scripts/."""
skill = _make_skill_with_scripts(
"test_skill",
{
"run.py": models.Script(
src="from helper import message\nprint(message())"
),
"helper.py": models.Script(
src="def message():\n return 'hello from helper'"
),
},
)
toolset = _make_real_executor_toolset([skill])
tool = skill_toolset.RunSkillScriptTool(toolset)
ctx = _make_tool_context_with_agent()
result = await tool.run_async(
args={
"skill_name": "test_skill",
"file_path": "run.py",
},
tool_context=ctx,
)
assert "status" in result, f"Result missing status: {result}"
assert result["status"] == "success"
assert result["stdout"] == "hello from helper\n"
assert result["stderr"] == ""


@pytest.mark.asyncio
async def test_integration_python_sys_exit_zero():
"""Real executor: sys.exit(0) is treated as success."""
Expand Down