Skip to content

fix(tools): hide skill script tool when no scripts exist - #6300

Open
Oxygen56 wants to merge 4 commits into
google:mainfrom
Oxygen56:fix-skill-toolset-script-tool-exposure
Open

fix(tools): hide skill script tool when no scripts exist#6300
Oxygen56 wants to merge 4 commits into
google:mainfrom
Oxygen56:fix-skill-toolset-script-tool-exposure

Conversation

@Oxygen56

Copy link
Copy Markdown

Link to Issue or Description of Change

1. Link to an existing issue (if applicable):

Problem:
SkillToolset always exposed run_skill_script, even when the configured local skills had no scripts. That gives the model an unavailable action surface and can contribute to repeated failed script calls.

Solution:
Only register RunSkillScriptTool when at least one local skill exposes scripts. Registry-backed toolsets keep the previous behavior because remote skill contents are not known at initialization time.

Testing Plan

Unit Tests:

  • I have added or updated unit tests for my change.
  • All unit tests pass locally.

Passed locally:

uv run --extra test pytest tests/unittests/tools/test_skill_toolset.py
# 115 passed, 4 warnings
uv run --extra dev ruff check src/google/adk/tools/skill_toolset.py tests/unittests/tools/test_skill_toolset.py
# All checks passed

Manual End-to-End (E2E) Tests:

Not run. The change is covered by isolated unit tests for the tool list returned by SkillToolset.

Checklist

  • I have read the CONTRIBUTING.md document.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have added tests that prove my fix is effective or that my feature works.
  • New and existing unit tests pass locally with my changes.
  • I have manually tested my changes end-to-end.
  • Any dependent changes have been merged and published in downstream modules.

Additional context

This keeps the script execution tool out of the model-visible tool list when there is no local script to run, while preserving registry behavior for dynamically loaded remote skills.

@rohityanrohityan added the tools [Component] This issue is related to tools label Jul 6, 2026
@rohityanrohityan added the needs review [Status] The PR/issue is awaiting review from the maintainer label Jul 8, 2026
@rohityan
rohityan requested a review from wyf7107July 8, 2026 22:05
@rohityan

Copy link
Copy Markdown
Collaborator

Hi @Oxygen56 , 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

Copy link
Copy Markdown
Collaborator

Hi @wyf7107 , can you please review this. LGTM.

@jazhang00jazhang00 self-assigned this Jul 14, 2026
@Oxygen56

Copy link
Copy Markdown
Author

Updated the BigQuery skill integration test to expect the three tools exposed when the skill has no scripts, matching this PR behavior.

Verification:

  • uv run --python 3.11 --with pytest --with pytest-asyncio --with pytest-mock pytest tests/unittests/tools/bigquery/test_bigquery_skill.py tests/unittests/tools/test_skill_toolset.py -q -> 122 passed
  • uvx pre-commit run --files tests/unittests/tools/bigquery/test_bigquery_skill.py -> passed

@saransh-translucent

Copy link
Copy Markdown

+1, we're hitting this in production with google-adk 2.7.0. Downstream repro adds a bit of signal to the report in #6281:

Scenario. Our on-disk skills (workbook editor, various document editors, a few querying skills) all ship SKILL.md and references/ only — none have a scripts/ directory. Over a recent 7-day window in one nonprod environment we saw 181/181run_skill_script calls return SCRIPT_NOT_FOUND. That would be merely wasteful, except ADK's own rule 6 in the injected skill system instruction tells the model:

"If run_skill_script returns an error, do not retry, report the error to the user and stop."

So a Claude Sonnet 4.6 turn that (a) writes some inline Python for the sandbox, (b) confidently closes by calling run_skill_script(skill_name="workbook-editor", file_path="scripts/setup.py") — because the instruction taught it to — hits SCRIPT_NOT_FOUND, obeys rule 6, and ends the turn silent. Our downstream side-effect gate ("did the sandbox emit WORKBOOK_COMMIT on stdout?") never fires because the inline Python that would have run it never got dispatched, so the user sees an empty message and no staged workbook proposal.

Why the tool_filter workaround from #6281 wasn't enough for us. We tried it. Filtering run_skill_script from get_tools() while leaving the injected system instruction intact makes it worse in ADK 2.5: the model still attempts the tool (the prompt still names it), the call falls through the client-side dispatch, and (per #6448) the run crashes with ValueError: Tool 'run_skill_script' not found. So we're currently carrying both fixes ourselves: tool_filter plus a subclass that overrides process_llm_request to inject a variant of _build_skill_system_instruction() with rules 4 (script use) and 6 (script error handling) removed and downstream rules renumbered. ~60 lines that we'd love to delete once this PR (and #6448) ships.

On the PR itself: the "register RunSkillScriptTool only when at least one local skill exposes scripts" heuristic looks right for our case — none of our skills would trigger it, and the ones that do (registry-backed remote skills) preserve current behavior. Happy to test a locally-patched ADK build against our nonprod eval harness (Claude Sonnet 4.6 + workbook-editing turns, exactly the failure mode above) if that'd help move the review.

Downstream tracking: our issue AI-2276 and the current subclass in translucent-chat#288 if it's useful as a reference for what the workaround looks like in ADK 2.7.

Sign up for freeto 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 maintainertools[Component] This issue is related to tools

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SkillToolset exposes run_skill_script tool causing hallucination loop when skill has no scripts/ directory

4 participants

@Oxygen56@rohityan@saransh-translucent@jazhang00