Skip to content

fix(skills): Make SkillToolset system instruction respect tool_filter#6449

Open
nicobytes wants to merge 3 commits into
google:mainfrom
nicobytes:fix/skill-toolset-instruction-tool-filter
Open

fix(skills): Make SkillToolset system instruction respect tool_filter#6449
nicobytes wants to merge 3 commits into
google:mainfrom
nicobytes:fix/skill-toolset-instruction-tool-filter

Conversation

@nicobytes

@nicobytes nicobytes commented Jul 22, 2026

Copy link
Copy Markdown

Summary

  • Build the skill system instruction from tools actually selected by tool_filter (list or predicate)
  • Stop advertising run_skill_script / load_skill_resource when filtered out, and explicitly tell the model not to call them — reply as normal text instead
  • Also keep the rest of process_llm_request consistent with the filter: inject skills XML when list_skills is filtered out, and omit the search_skills registry hint when that tool is filtered out
  • Leave tool registration unchanged (related follow-up: SkillToolset exposes run_skill_script tool causing hallucination loop when skill has no scripts/ directory #6281)

Why

get_tools() already respected tool_filter, but process_llm_request always injected _build_skill_system_instruction(), which unconditionally told the model to use run_skill_script and load_skill_resource. With instruction-only skills (tool_filter=["list_skills", "load_skill"]), Gemini often wrapped the final reply in a hallucinated run_skill_script call → UNEXPECTED_TOOL_CALL / ValueError: Tool 'run_skill_script' not found. and an empty user-facing response.

Fixes #6448
Related to #6281

Testing Plan

  • Unit: filtered instruction omits positive usage of script/resource tools and adds an explicit ban
  • Unit: unfiltered DEFAULT_SKILL_SYSTEM_INSTRUCTION contract unchanged
  • Unit: process_llm_request with tool_filter=["list_skills", "load_skill"] injects filtered instruction
  • Unit: excluding list_skills injects skills XML fallback
  • Unit: registry + filter excluding/including search_skills controls the registry hint
  • Unit: tool_name_prefix + filter uses prefixed names in instruction/ban
  • Unit: ToolPredicate filter is respected by the injected instruction
  • Existing test_skill_toolset.py suite green
uv run pytest ./tests/unittests/tools/test_skill_toolset.py -q
# 123 passed
uv run pre-commit run --files src/google/adk/tools/skill_toolset.py tests/unittests/tools/test_skill_toolset.py
# all hooks Passed

Manual repro (pre-fix)

from google.adk import Agent
from google.adk.skills import models
from google.adk.tools.skill_toolset import SkillToolset

skill = models.Skill(
    frontmatter=models.Frontmatter(
        name="whatsapp-formatting",
        description="Format messages for WhatsApp native rendering.",
    ),
    instructions="Use *bold* and _italic_ WhatsApp syntax. No scripts.",
)

toolset = SkillToolset(
    skills=[skill],
    tool_filter=["list_skills", "load_skill"],
)

agent = Agent(
    model="gemini-flash-latest",
    name="demo",
    instruction="Help the user. Load the formatting skill when needed, then reply as text.",
    tools=[toolset],
)

Before: model often calls run_skill_script after load_skill → crash.
After: instruction no longer advertises filtered tools and tells the model to answer as normal text.

@google-cla

google-cla Bot commented Jul 22, 2026

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@adk-bot adk-bot added the tools [Component] This issue is related to tools label Jul 22, 2026
@nicobytes

Copy link
Copy Markdown
Author

recheck

@nicobytes
nicobytes force-pushed the fix/skill-toolset-instruction-tool-filter branch from 9795561 to 8daac80 Compare July 22, 2026 15:35
process_llm_request always advertised run_skill_script and
load_skill_resource even when tool_filter excluded them, causing the
model to call undeclared tools and crash with ValueError.

Build the injected system instruction from selected tools only, and
explicitly forbid filtered-out script/resource tools.

Fixes google#6448
@nicobytes
nicobytes force-pushed the fix/skill-toolset-instruction-tool-filter branch from 8daac80 to 8977e84 Compare July 22, 2026 15:36
@rohityan

Copy link
Copy Markdown
Collaborator

Hi @nicobytes , Thank you for your contribution! We appreciate you taking the time to submit this pull request. Your PR has been received by the team and is currently under review. We will provide feedback as soon as we have an update to share.

@rohityan rohityan added the needs review [Status] The PR/issue is awaiting review from the maintainer label Jul 22, 2026
@rohityan
rohityan requested a review from sasha-gitg July 22, 2026 22:49
@rohityan

Copy link
Copy Markdown
Collaborator

Hi @sasha-gitg , can you please review this. LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs review [Status] The PR/issue is awaiting review from the maintainer tools [Component] This issue is related to tools

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SkillToolset ignores tool_filter when injecting system instruction (mentions run_skill_script / load_skill_resource even when filtered out)

4 participants