Skip to content

Commit 8977e84

Browse files
committed
fix(skills): Make SkillToolset instruction respect tool_filter
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 #6448
1 parent 2316b83 commit 8977e84

2 files changed

Lines changed: 345 additions & 33 deletions

File tree

src/google/adk/tools/skill_toolset.py

Lines changed: 174 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -59,10 +59,72 @@
5959
)
6060

6161

62-
def _build_skill_system_instruction(prefix: str | None = None) -> str:
62+
def _build_skill_system_instruction(
63+
prefix: str | None = None,
64+
allowed_tools: set[str] | frozenset[str] | None = None,
65+
) -> str:
66+
"""Builds the skill system instruction for the LLM.
67+
68+
Args:
69+
prefix: Optional prefix prepended to tool names in the instruction.
70+
allowed_tools: Optional set of base tool names available after filtering.
71+
When None, documents all skill tools (historical default, used for
72+
``DEFAULT_SKILL_SYSTEM_INSTRUCTION``). When provided, only documents
73+
tools present in the set and explicitly forbids calling filtered-out
74+
script/resource tools.
75+
"""
6376
p = f"{prefix}_" if prefix else ""
6477

65-
return (
78+
# Preserve the historical full instruction verbatim when no filter is
79+
# applied so DEFAULT_SKILL_SYSTEM_INSTRUCTION and existing equality
80+
# assertions stay byte-identical.
81+
if allowed_tools is None:
82+
return (
83+
"You can use specialized 'skills' to help you with complex tasks. "
84+
"You MUST use the skill tools to interact with these skills.\n\n"
85+
"Skills are folders of instructions and resources that extend your "
86+
"capabilities for specialized tasks. Each skill folder contains:\n"
87+
"- **SKILL.md** (required): The main instruction file with skill "
88+
"metadata and detailed markdown instructions.\n"
89+
"- **references/** (Optional): Additional documentation or examples"
90+
" for "
91+
"skill usage.\n"
92+
"- **assets/** (Optional): Templates, scripts or other resources"
93+
" used by "
94+
"the skill.\n"
95+
"- **scripts/** (Optional): Executable scripts that can be run via "
96+
"bash.\n\n"
97+
"This is very important:\n\n"
98+
"1. If a skill seems relevant to the current user query, you MUST use "
99+
f'the `{p}load_skill` tool with `skill_name="<SKILL_NAME>"` to read '
100+
"its full instructions before proceeding.\n"
101+
"2. Once you have read the instructions, follow them exactly as "
102+
"documented before replying to the user. For example, If the "
103+
"instruction lists multiple steps, please make sure you complete all "
104+
"of them in order.\n"
105+
f"3. The `{p}load_skill_resource` tool is for viewing files within a "
106+
"skill's directory (e.g., `references/*`, `assets/*`, `scripts/*`). "
107+
"It is ONLY for skill-bundled files — do NOT use it to access "
108+
"documents or files provided by the user at runtime. Do NOT use "
109+
"other tools to access skill files.\n"
110+
f"4. Use `{p}run_skill_script` to run scripts from a skill's"
111+
" `scripts/` "
112+
f"directory. Use `{p}load_skill_resource` to view script content"
113+
" first if "
114+
"needed.\n"
115+
f"5. If `{p}load_skill_resource` returns any error, do not retry any "
116+
"path. Report the error to the user and stop.\n"
117+
f"6. If `{p}run_skill_script` returns an error (for example "
118+
f"`SCRIPT_NOT_FOUND`), do not retry the same script or guess a "
119+
"different script path. Report the error to the user and stop.\n"
120+
f"7. Loading a skill only retrieves its instructions; it does NOT "
121+
f"complete your turn. After a `{p}load_skill` call returns, continue "
122+
"in the SAME turn: call whatever tools the skill's steps require "
123+
"(search, data retrieval, render), then write your reply. Never end "
124+
"your turn with an empty response right after loading a skill.\n"
125+
)
126+
127+
intro = (
66128
"You can use specialized 'skills' to help you with complex tasks. "
67129
"You MUST use the skill tools to interact with these skills.\n\n"
68130
"Skills are folders of instructions and resources that extend your "
@@ -76,34 +138,99 @@ def _build_skill_system_instruction(prefix: str | None = None) -> str:
76138
"- **scripts/** (Optional): Executable scripts that can be run via "
77139
"bash.\n\n"
78140
"This is very important:\n\n"
79-
"1. If a skill seems relevant to the current user query, you MUST use "
80-
f'the `{p}load_skill` tool with `skill_name="<SKILL_NAME>"` to read '
81-
"its full instructions before proceeding.\n"
82-
"2. Once you have read the instructions, follow them exactly as "
83-
"documented before replying to the user. For example, If the "
84-
"instruction lists multiple steps, please make sure you complete all "
85-
"of them in order.\n"
86-
f"3. The `{p}load_skill_resource` tool is for viewing files within a "
87-
"skill's directory (e.g., `references/*`, `assets/*`, `scripts/*`). "
88-
"It is ONLY for skill-bundled files — do NOT use it to access "
89-
"documents or files provided by the user at runtime. Do NOT use "
90-
"other tools to access skill files.\n"
91-
f"4. Use `{p}run_skill_script` to run scripts from a skill's `scripts/` "
92-
f"directory. Use `{p}load_skill_resource` to view script content"
93-
" first if "
94-
"needed.\n"
95-
f"5. If `{p}load_skill_resource` returns any error, do not retry any "
96-
"path. Report the error to the user and stop.\n"
97-
f"6. If `{p}run_skill_script` returns an error (for example "
98-
f"`SCRIPT_NOT_FOUND`), do not retry the same script or guess a "
99-
"different script path. Report the error to the user and stop.\n"
100-
f"7. Loading a skill only retrieves its instructions; it does NOT "
101-
f"complete your turn. After a `{p}load_skill` call returns, continue "
102-
"in the SAME turn: call whatever tools the skill's steps require "
103-
"(search, data retrieval, render), then write your reply. Never end "
104-
"your turn with an empty response right after loading a skill.\n"
105141
)
106142

143+
has_load = "load_skill" in allowed_tools
144+
has_resource = "load_skill_resource" in allowed_tools
145+
has_script = "run_skill_script" in allowed_tools
146+
147+
steps: list[str] = []
148+
step = 1
149+
150+
if has_load:
151+
steps.append(
152+
f"{step}. If a skill seems relevant to the current user query, you"
153+
" MUST use "
154+
f'the `{p}load_skill` tool with `skill_name="<SKILL_NAME>"` to read '
155+
"its full instructions before proceeding.\n"
156+
)
157+
step += 1
158+
steps.append(
159+
f"{step}. Once you have read the instructions, follow them exactly"
160+
" as documented before replying to the user. For example, If the "
161+
"instruction lists multiple steps, please make sure you complete all "
162+
"of them in order.\n"
163+
)
164+
step += 1
165+
166+
if has_resource:
167+
steps.append(
168+
f"{step}. The `{p}load_skill_resource` tool is for viewing files"
169+
" within a skill's directory (e.g., `references/*`, `assets/*`,"
170+
" `scripts/*`). It is ONLY for skill-bundled files — do NOT use it"
171+
" to access documents or files provided by the user at runtime. Do"
172+
" NOT use other tools to access skill files.\n"
173+
)
174+
step += 1
175+
176+
if has_script:
177+
if has_resource:
178+
script_step = (
179+
f"{step}. Use `{p}run_skill_script` to run scripts from a skill's"
180+
f" `scripts/` directory. Use `{p}load_skill_resource` to view"
181+
" script content first if needed.\n"
182+
)
183+
else:
184+
script_step = (
185+
f"{step}. Use `{p}run_skill_script` to run scripts from a skill's"
186+
" `scripts/` directory.\n"
187+
)
188+
steps.append(script_step)
189+
step += 1
190+
191+
if has_resource:
192+
steps.append(
193+
f"{step}. If `{p}load_skill_resource` returns any error, do not"
194+
" retry any path. Report the error to the user and stop.\n"
195+
)
196+
step += 1
197+
198+
if has_script:
199+
steps.append(
200+
f"{step}. If `{p}run_skill_script` returns an error (for example "
201+
"`SCRIPT_NOT_FOUND`), do not retry the same script or guess a "
202+
"different script path. Report the error to the user and stop.\n"
203+
)
204+
step += 1
205+
206+
if has_load:
207+
steps.append(
208+
f"{step}. Loading a skill only retrieves its instructions; it does"
209+
f" NOT complete your turn. After a `{p}load_skill` call returns,"
210+
" continue in the SAME turn: call whatever tools the skill's steps"
211+
" require (search, data retrieval, render), then write your reply."
212+
" Never end your turn with an empty response right after loading a"
213+
" skill.\n"
214+
)
215+
step += 1
216+
217+
banned: list[str] = []
218+
if not has_script:
219+
banned.append(f"`{p}run_skill_script`")
220+
if not has_resource:
221+
banned.append(f"`{p}load_skill_resource`")
222+
if banned:
223+
banned_csv = ", ".join(banned)
224+
steps.append(
225+
f"{step}. The following tools are NOT available: {banned_csv}."
226+
" Do NOT call them. After loading a skill (if available), apply"
227+
" its instructions in context and write your final reply as"
228+
" normal model text. Never wrap the user-facing answer inside a"
229+
" tool call.\n"
230+
)
231+
232+
return intro + "".join(steps)
233+
107234

108235
class ListSkillsTool(BaseTool):
109236
"""Tool to list all available skills."""
@@ -1190,18 +1317,32 @@ async def process_llm_request(
11901317
self, *, tool_context: ToolContext, llm_request: LlmRequest
11911318
) -> None:
11921319
"""Processes the outgoing LLM request to include available skills."""
1193-
instructions = [
1194-
_build_skill_system_instruction(prefix=self.tool_name_prefix)
1195-
]
1320+
selected_core_tools = {
1321+
t.name for t in self._tools if self._is_tool_selected(t, tool_context)
1322+
}
1323+
1324+
# Only pass allowed_tools when a filter is configured so the unfiltered
1325+
# path keeps the historical DEFAULT_SKILL_SYSTEM_INSTRUCTION text.
1326+
if self.tool_filter is None:
1327+
instructions = [
1328+
_build_skill_system_instruction(prefix=self.tool_name_prefix)
1329+
]
1330+
else:
1331+
instructions = [
1332+
_build_skill_system_instruction(
1333+
prefix=self.tool_name_prefix,
1334+
allowed_tools=selected_core_tools,
1335+
)
1336+
]
11961337

1197-
has_list_skills = any(isinstance(t, ListSkillsTool) for t in self._tools)
1338+
has_list_skills = "list_skills" in selected_core_tools
11981339

11991340
if not has_list_skills:
12001341
skills = self._list_skills()
12011342
skills_xml = prompt.format_skills_as_xml(skills)
12021343
instructions.append(skills_xml)
12031344

1204-
if self._registry:
1345+
if self._registry and "search_skills" in selected_core_tools:
12051346
p = f"{self.tool_name_prefix}_" if self.tool_name_prefix else ""
12061347
instructions.append(
12071348
"\nIf the locally available skills are not sufficient to complete "

0 commit comments

Comments
 (0)