diff --git a/python/packages/core/agent_framework/_skills.py b/python/packages/core/agent_framework/_skills.py index aeee8df6476..24f534fb692 100644 --- a/python/packages/core/agent_framework/_skills.py +++ b/python/packages/core/agent_framework/_skills.py @@ -2318,8 +2318,13 @@ async def _run_skill_script( ``agent.run(user_id="123")``). Returns: - The result, or a user-facing error message on - failure. + The script result. Returns a user-facing error string for + validation failures (empty or unknown skill/script name). + + Raises: + Exception: Re-raises any exception raised while running the script, + delegating error handling to the function-invocation pipeline + (which applies its own ``include_detailed_errors`` policy). """ if not skill_name or not skill_name.strip(): return "Error: Skill name cannot be empty." @@ -2339,7 +2344,7 @@ async def _run_skill_script( return await script.run(skill, args, **kwargs) except Exception: logger.exception("Error running script '%s' in skill '%s'", script_name, skill_name) - return f"Error: Failed to run script '{script_name}' in skill '{skill_name}'." + raise async def _read_skill_resource( self, skills: Sequence[Skill], skill_name: str, resource_name: str, **kwargs: Any @@ -2359,8 +2364,16 @@ async def _read_skill_resource( ``agent.run(user_id="123")``). Returns: - The resource content (any type), or a user-facing error message on - failure. + The resource content (any type). Returns a user-facing error + string for validation failures (empty or unknown skill/resource + name). + + Raises: + Exception: Re-raises any exception raised while reading the + resource. Resources take no model-supplied arguments, so a + swallowed generic error is not actionable by the model; + re-raising lets the function-invocation pipeline decide how to + surface it. """ if not skill_name or not skill_name.strip(): return "Error: Skill name cannot be empty." @@ -2380,7 +2393,7 @@ async def _read_skill_resource( return await resource.read(**kwargs) except Exception: logger.exception("Failed to read resource '%s' from skill '%s'", resource_name, skill_name) - return f"Error: Failed to read resource '{resource_name}' from skill '{skill_name}'." + raise # endregion diff --git a/python/packages/core/tests/core/test_skills.py b/python/packages/core/tests/core/test_skills.py index d9789709f77..63936e15220 100644 --- a/python/packages/core/tests/core/test_skills.py +++ b/python/packages/core/tests/core/test_skills.py @@ -2802,7 +2802,7 @@ async def test_read_skill_resource_whitespace_resource_name_returns_error(self) assert result.startswith("Error:") assert "empty" in result - async def test_read_callable_resource_exception_returns_error(self) -> None: + async def test_read_callable_resource_exception_propagates(self) -> None: skill = InlineSkill(frontmatter=SkillFrontmatter(name="my-skill", description="A skill."), instructions="Body") @skill.resource @@ -2811,11 +2811,10 @@ def exploding_resource() -> Any: provider = SkillsProvider([skill]) await _init_provider(provider) - result = await provider._read_skill_resource(_raw_skills(provider), "my-skill", "exploding_resource") - assert result.startswith("Error:") - assert "Failed to read resource" in result + with pytest.raises(RuntimeError, match="boom"): + await provider._read_skill_resource(_raw_skills(provider), "my-skill", "exploding_resource") - async def test_read_async_callable_resource_exception_returns_error(self) -> None: + async def test_read_async_callable_resource_exception_propagates(self) -> None: skill = InlineSkill(frontmatter=SkillFrontmatter(name="my-skill", description="A skill."), instructions="Body") @skill.resource @@ -2824,8 +2823,8 @@ async def async_exploding() -> Any: provider = SkillsProvider([skill]) await _init_provider(provider) - result = await provider._read_skill_resource(_raw_skills(provider), "my-skill", "async_exploding") - assert result.startswith("Error:") + with pytest.raises(ValueError, match="async boom"): + await provider._read_skill_resource(_raw_skills(provider), "my-skill", "async_exploding") async def test_load_code_skill_xml_escapes_metadata(self) -> None: skill = InlineSkill( @@ -3588,10 +3587,9 @@ async def test_file_script_error_without_runner(self) -> None: result = await run_tool.func(skill_name="my-skill", script_name="code-s") assert result == "ok" - # File script without runner returns error - result = await run_tool.func(skill_name="my-skill", script_name="file-s") - assert "Error" in result - assert "Failed to run" in result + # File script without runner propagates an error by default + with pytest.raises(TypeError, match="requires a FileSkill"): + await run_tool.func(skill_name="my-skill", script_name="file-s") async def test_async_code_script_runs_directly(self) -> None: async def async_func(x: int = 0) -> str: @@ -3646,10 +3644,9 @@ async def test_script_with_path_errors_without_runner(self) -> None: result = await run_tool.func(skill_name="my-skill", script_name="code-s") assert result == "ok" - # Path+function script without runner returns error - result = await run_tool.func(skill_name="my-skill", script_name="path-s") - assert "Error" in result - assert "script_runner" in result or "Failed to run" in result + # Path+function script without runner propagates an error by default + with pytest.raises(TypeError, match="requires a FileSkill"): + await run_tool.func(skill_name="my-skill", script_name="path-s") async def test_run_skill_script_error_on_missing_skill(self) -> None: skill = InlineSkill(frontmatter=SkillFrontmatter(name="my-skill", description="test"), instructions="body") @@ -3717,10 +3714,10 @@ def process(**kwargs: Any) -> str: provider = SkillsProvider([skill]) await _init_provider(provider) - result = await provider._run_skill_script( - _raw_skills(provider), "my-skill", "process", args={"mode": "llm-value"}, mode="runtime-value" - ) - assert "Error" in result + with pytest.raises(TypeError): + await provider._run_skill_script( + _raw_skills(provider), "my-skill", "process", args={"mode": "llm-value"}, mode="runtime-value" + ) async def test_run_skill_script_error_on_missing_script(self) -> None: skill = InlineSkill(frontmatter=SkillFrontmatter(name="my-skill", description="test"), instructions="body") @@ -3804,8 +3801,8 @@ async def test_require_script_approval_does_not_affect_other_tools(self) -> None for t in other_tools: assert t.approval_mode == "never_require" - async def test_code_script_exception_returns_error(self) -> None: - """A code script function that raises should return an error string.""" + async def test_code_script_exception_propagates_by_default(self) -> None: + """A code script function that raises should propagate by default.""" def failing_script() -> str: raise RuntimeError("Something went wrong") @@ -3816,10 +3813,8 @@ def failing_script() -> str: provider = SkillsProvider([skill]) await _init_provider(provider) run_tool = next(t for t in _ctx(provider)[2] if hasattr(t, "name") and t.name == "run_skill_script") - result = await run_tool.func(skill_name="my-skill", script_name="boom") - assert "Error" in result - assert "boom" in result - assert "Something went wrong" not in result + with pytest.raises(RuntimeError, match="Something went wrong"): + await run_tool.func(skill_name="my-skill", script_name="boom") async def test_custom_template_without_runner_placeholder_raises(self) -> None: """Providers accept custom templates without {runner_instructions}.""" @@ -5799,17 +5794,16 @@ def runner(skill: Any, script: Any, args: Any = None) -> str: assert result == "list_result" assert captured["args"] == ["input.docx", "--verbose"] - async def test_run_skill_script_inline_with_list_args_returns_error(self) -> None: - """Inline script called with list args through provider returns error (TypeError caught).""" + async def test_run_skill_script_inline_with_list_args_propagates_error(self) -> None: + """Inline script called with list args through provider propagates the TypeError by default.""" skill = InlineSkill(frontmatter=SkillFrontmatter(name="my-skill", description="test"), instructions="body") skill._scripts.append(InlineSkillScript(name="s1", function=lambda: "ok")) provider = SkillsProvider([skill]) await _init_provider(provider) run_tool = next(t for t in _ctx(provider)[2] if hasattr(t, "name") and t.name == "run_skill_script") - result = await run_tool.func(skill_name="my-skill", script_name="s1", args=["arg1"]) - assert "Error" in result - assert "Failed to run" in result + with pytest.raises(TypeError, match="requires keyword arguments"): + await run_tool.func(skill_name="my-skill", script_name="s1", args=["arg1"]) async def test_file_skill_content_includes_scripts_block(self) -> None: """FileSkill.content appends an block when scripts are present."""