Skip to content

Python: Stop swallowing skill script and resource errors so the model can self-correct #6755

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
giles17 wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Python: Stop swallowing skill script and resource errors so the model can self-correct #6755

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5fce632
Python: Add include_detailed_errors option for skill script execution
giles17 Jun 26, 2026
c32df61
Re-raise skill script/resource errors instead of adding a provider op…
giles17 Jun 26, 2026
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
25 changes: 19 additions & 6 deletions python/packages/core/agent_framework/_skills.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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."
Expand All @@ -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
Expand All @@ -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."
Expand All @@ -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
Expand Down
54 changes: 24 additions & 30 deletions python/packages/core/tests/core/test_skills.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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(
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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")
Expand All @@ -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}."""
Expand Down Expand Up @@ -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 <available_scripts> block when scripts are present."""
Expand Down
Loading