Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
18 changes: 17 additions & 1 deletion src/google/adk/tools/skill_toolset.py
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,17 @@ async def run_async(
activated_skills = list(tool_context.state.get(state_key) or [])
if skill_name not in activated_skills:
activated_skills.append(skill_name)
tool_context.state[state_key] = activated_skills
else:
# Move to the end (most recently used) for rolling window.
activated_skills.remove(skill_name)
activated_skills.append(skill_name)

# Enforce rolling window: evict oldest skills if limit is exceeded.
max_skills = self._toolset._max_activated_skills
if max_skills is not None and len(activated_skills) > max_skills:
activated_skills = activated_skills[-max_skills:]

tool_context.state[state_key] = activated_skills

return {
"skill_name": skill_name,
Expand Down Expand Up @@ -886,6 +896,7 @@ def __init__(
code_executor: BaseCodeExecutor | None = None,
script_timeout: int = _DEFAULT_SCRIPT_TIMEOUT,
additional_tools: list[ToolUnion] | None = None,
max_activated_skills: int | None = None,
):
"""Initializes the SkillToolset.

Expand All @@ -898,6 +909,10 @@ def __init__(
scripts executed via exec().
additional_tools: Optional list of `BaseTool` or `BaseToolset` instances
to be made available to the agent when certain skills are activated.
max_activated_skills: Optional maximum number of skills to keep activated
simultaneously. When set, only the most recently activated skills are
retained (rolling window). Older skills and their associated tools are
evicted from the session state. Defaults to None (no limit).
"""
super().__init__()

Expand All @@ -914,6 +929,7 @@ def __init__(
self._registry = registry
self._code_executor = code_executor
self._script_timeout = script_timeout
self._max_activated_skills = max_activated_skills
# Needed for mid-turn reloading of skill tools.
self._use_invocation_cache = False
# Cache fetched remote skill definitions per turn to reduce requests to registry
Expand Down
339 changes: 339 additions & 0 deletions tests/unittests/tools/test_skill_toolset.py
Original file line number Diff line number Diff line change
Expand Up @@ -1925,3 +1925,342 @@ async def test_close_cancels_futures_and_clears_cache():
assert fut1.cancelled()
assert not fut2.cancelled() # Done futures shouldn't/can't be cancelled
assert not toolset._fetched_skill_cache


# --- Tests for max_activated_skills (rolling window) ---


@pytest.mark.asyncio
async def test_max_activated_skills_evicts_oldest(
mock_skill1, mock_skill2, tool_context_instance
):
"""When max_activated_skills is exceeded, the oldest skill is evicted."""
toolset = skill_toolset.SkillToolset(
[mock_skill1, mock_skill2], max_activated_skills=1
)
tool = skill_toolset.LoadSkillTool(toolset)

state_key = "_adk_activated_skill_test_agent"

# Load skill1
tool_context_instance.state.get.return_value = None
await tool.run_async(
args={"skill_name": "skill1"}, tool_context=tool_context_instance
)
tool_context_instance.state.__setitem__.assert_called_with(
state_key, ["skill1"]
)

# Load skill2 — skill1 should be evicted
tool_context_instance.state.get.return_value = ["skill1"]
await tool.run_async(
args={"skill_name": "skill2"}, tool_context=tool_context_instance
)
tool_context_instance.state.__setitem__.assert_called_with(
state_key, ["skill2"]
)


@pytest.mark.asyncio
async def test_max_activated_skills_keeps_recent_n(
tool_context_instance,
):
"""Rolling window keeps the N most recently activated skills."""
skills = []
for i in range(5):
skill = mock.create_autospec(models.Skill, instance=True)
skill.name = f"skill_{i}"
skill.instructions = f"instructions for skill_{i}"
frontmatter = mock.create_autospec(models.Frontmatter, instance=True)
frontmatter.name = f"skill_{i}"
frontmatter.metadata = {}
frontmatter.model_dump.return_value = {"name": f"skill_{i}"}
skill.frontmatter = frontmatter
skills.append(skill)

toolset = skill_toolset.SkillToolset(skills, max_activated_skills=3)
tool = skill_toolset.LoadSkillTool(toolset)

state_key = "_adk_activated_skill_test_agent"

# Sequentially load skills 0 through 4
current_state = []
for i in range(5):
tool_context_instance.state.get.return_value = list(current_state)
await tool.run_async(
args={"skill_name": f"skill_{i}"}, tool_context=tool_context_instance
)
# Capture what was set
call_args = tool_context_instance.state.__setitem__.call_args
current_state = call_args[0][1]

# After loading 5 skills with max=3, only the last 3 should remain
assert current_state == ["skill_2", "skill_3", "skill_4"]


@pytest.mark.asyncio
async def test_max_activated_skills_reloading_moves_to_end(
mock_skill1, mock_skill2, tool_context_instance
):
"""Re-loading an already active skill moves it to the end (most recent)."""
# Create a third skill
mock_skill3 = mock.create_autospec(models.Skill, instance=True)
mock_skill3.name = "skill3"
mock_skill3.instructions = "instructions for skill3"
frontmatter3 = mock.create_autospec(models.Frontmatter, instance=True)
frontmatter3.name = "skill3"
frontmatter3.metadata = {}
frontmatter3.model_dump.return_value = {"name": "skill3"}
mock_skill3.frontmatter = frontmatter3

toolset = skill_toolset.SkillToolset(
[mock_skill1, mock_skill2, mock_skill3], max_activated_skills=3
)
tool = skill_toolset.LoadSkillTool(toolset)

state_key = "_adk_activated_skill_test_agent"

# Start with all 3 loaded
tool_context_instance.state.get.return_value = [
"skill1",
"skill2",
"skill3",
]

# Reload skill1 — should move to end
await tool.run_async(
args={"skill_name": "skill1"}, tool_context=tool_context_instance
)
call_args = tool_context_instance.state.__setitem__.call_args
assert call_args[0][1] == ["skill2", "skill3", "skill1"]


@pytest.mark.asyncio
async def test_no_max_activated_skills_unlimited(
tool_context_instance,
):
"""Without max_activated_skills, all skills are retained (no eviction)."""
skills = []
for i in range(10):
skill = mock.create_autospec(models.Skill, instance=True)
skill.name = f"skill_{i}"
skill.instructions = f"instructions for skill_{i}"
frontmatter = mock.create_autospec(models.Frontmatter, instance=True)
frontmatter.name = f"skill_{i}"
frontmatter.metadata = {}
frontmatter.model_dump.return_value = {"name": f"skill_{i}"}
skill.frontmatter = frontmatter
skills.append(skill)

toolset = skill_toolset.SkillToolset(skills) # No max_activated_skills
tool = skill_toolset.LoadSkillTool(toolset)

state_key = "_adk_activated_skill_test_agent"

current_state = []
for i in range(10):
tool_context_instance.state.get.return_value = list(current_state)
await tool.run_async(
args={"skill_name": f"skill_{i}"}, tool_context=tool_context_instance
)
call_args = tool_context_instance.state.__setitem__.call_args
current_state = call_args[0][1]

# All 10 skills should be retained
assert len(current_state) == 10
assert current_state == [f"skill_{i}" for i in range(10)]


@pytest.mark.asyncio
async def test_evicted_skill_tools_not_resolved():
"""After a skill is evicted, its additional tools are no longer resolved."""
# Create two skills, each declaring different additional tools
skill_a = mock.create_autospec(models.Skill, instance=True)
skill_a.name = "skill_a"
skill_a.instructions = "instructions A"
frontmatter_a = mock.create_autospec(models.Frontmatter, instance=True)
frontmatter_a.name = "skill_a"
frontmatter_a.metadata = {"adk_additional_tools": ["tool_alpha"]}
frontmatter_a.model_dump.return_value = {"name": "skill_a"}
skill_a.frontmatter = frontmatter_a

skill_b = mock.create_autospec(models.Skill, instance=True)
skill_b.name = "skill_b"
skill_b.instructions = "instructions B"
frontmatter_b = mock.create_autospec(models.Frontmatter, instance=True)
frontmatter_b.name = "skill_b"
frontmatter_b.metadata = {"adk_additional_tools": ["tool_beta"]}
frontmatter_b.model_dump.return_value = {"name": "skill_b"}
skill_b.frontmatter = frontmatter_b

# Create mock additional tools
tool_alpha = mock.create_autospec(skill_toolset.BaseTool, instance=True)
tool_alpha.name = "tool_alpha"
tool_beta = mock.create_autospec(skill_toolset.BaseTool, instance=True)
tool_beta.name = "tool_beta"

toolset = skill_toolset.SkillToolset(
[skill_a, skill_b],
additional_tools=[tool_alpha, tool_beta],
max_activated_skills=1,
)

# Simulate: skill_a was activated, then skill_b was activated (evicting A)
readonly_context = mock.create_autospec(ReadonlyContext, instance=True)
readonly_context.agent_name = "test_agent"
readonly_context.invocation_id = "inv-1"
# After eviction, only skill_b is in state
readonly_context.state.get.return_value = ["skill_b"]

tools = await toolset.get_tools(readonly_context=readonly_context)
tool_names = {t.name for t in tools}

# tool_beta should be present (skill_b is active)
assert "tool_beta" in tool_names
# tool_alpha should NOT be present (skill_a was evicted)
assert "tool_alpha" not in tool_names


@pytest.mark.asyncio
async def test_evicted_tool_call_raises_value_error():
"""Simulates the LLM calling a tool from an evicted skill.

This mirrors what happens in the real flow: after skill eviction, the
tool is removed from tools_dict, so _get_tool raises ValueError.
This test validates that the tools_dict correctly excludes evicted tools.
"""
# Skill with additional tool
skill_a = mock.create_autospec(models.Skill, instance=True)
skill_a.name = "skill_a"
skill_a.instructions = "instructions A"
frontmatter_a = mock.create_autospec(models.Frontmatter, instance=True)
frontmatter_a.name = "skill_a"
frontmatter_a.metadata = {"adk_additional_tools": ["expensive_api_tool"]}
frontmatter_a.model_dump.return_value = {"name": "skill_a"}
skill_a.frontmatter = frontmatter_a

skill_b = mock.create_autospec(models.Skill, instance=True)
skill_b.name = "skill_b"
skill_b.instructions = "instructions B"
frontmatter_b = mock.create_autospec(models.Frontmatter, instance=True)
frontmatter_b.name = "skill_b"
frontmatter_b.metadata = {}
frontmatter_b.model_dump.return_value = {"name": "skill_b"}
skill_b.frontmatter = frontmatter_b

expensive_tool = mock.create_autospec(skill_toolset.BaseTool, instance=True)
expensive_tool.name = "expensive_api_tool"

toolset = skill_toolset.SkillToolset(
[skill_a, skill_b],
additional_tools=[expensive_tool],
max_activated_skills=1,
)

# After eviction: only skill_b remains
readonly_context = mock.create_autospec(ReadonlyContext, instance=True)
readonly_context.agent_name = "test_agent"
readonly_context.invocation_id = "inv-2"
readonly_context.state.get.return_value = ["skill_b"]

tools = await toolset.get_tools(readonly_context=readonly_context)
tools_dict = {t.name: t for t in tools}

# The LLM might still try to call "expensive_api_tool" from history
assert "expensive_api_tool" not in tools_dict

# Simulate what _get_tool in functions.py would do
from google.genai import types as genai_types

fake_function_call = mock.MagicMock()
fake_function_call.name = "expensive_api_tool"

# This mirrors the behavior in flows/llm_flows/functions.py:_get_tool
with pytest.raises(ValueError, match="Tool 'expensive_api_tool' not found"):
from google.adk.flows.llm_flows.functions import _get_tool

_get_tool(fake_function_call, tools_dict)


@pytest.mark.asyncio
async def test_rolling_window_full_lifecycle_with_tool_resolution():
"""End-to-end: load skills, evict, verify tool availability at each step."""
skills = []
additional_tools = []
for i in range(4):
skill = mock.create_autospec(models.Skill, instance=True)
skill.name = f"skill_{i}"
skill.instructions = f"instructions {i}"
frontmatter = mock.create_autospec(models.Frontmatter, instance=True)
frontmatter.name = f"skill_{i}"
frontmatter.metadata = {"adk_additional_tools": [f"tool_{i}"]}
frontmatter.model_dump.return_value = {"name": f"skill_{i}"}
skill.frontmatter = frontmatter
skills.append(skill)

tool = mock.create_autospec(skill_toolset.BaseTool, instance=True)
tool.name = f"tool_{i}"
additional_tools.append(tool)

toolset = skill_toolset.SkillToolset(
skills,
additional_tools=additional_tools,
max_activated_skills=2,
)

load_tool = skill_toolset.LoadSkillTool(toolset)
ctx = mock.create_autospec(skill_toolset.ToolContext, instance=True)
ctx._invocation_context = mock.MagicMock()
ctx.agent_name = "test_agent"
ctx.invocation_id = "inv-1"

readonly_context = mock.create_autospec(ReadonlyContext, instance=True)
readonly_context.agent_name = "test_agent"
readonly_context.invocation_id = "inv-1"

# Step 1: Load skill_0 and skill_1
current_state = []
for i in range(2):
ctx.state.get.return_value = list(current_state)
await load_tool.run_async(
args={"skill_name": f"skill_{i}"}, tool_context=ctx
)
current_state = ctx.state.__setitem__.call_args[0][1]

assert current_state == ["skill_0", "skill_1"]

readonly_context.state.get.return_value = list(current_state)
tools = await toolset.get_tools(readonly_context=readonly_context)
tool_names = {t.name for t in tools}
assert "tool_0" in tool_names
assert "tool_1" in tool_names
assert "tool_2" not in tool_names

# Step 2: Load skill_2 — should evict skill_0
ctx.state.get.return_value = list(current_state)
await load_tool.run_async(args={"skill_name": "skill_2"}, tool_context=ctx)
current_state = ctx.state.__setitem__.call_args[0][1]

assert current_state == ["skill_1", "skill_2"]

readonly_context.state.get.return_value = list(current_state)
tools = await toolset.get_tools(readonly_context=readonly_context)
tool_names = {t.name for t in tools}
assert "tool_0" not in tool_names # evicted!
assert "tool_1" in tool_names
assert "tool_2" in tool_names

# Step 3: Load skill_3 — should evict skill_1
ctx.state.get.return_value = list(current_state)
await load_tool.run_async(args={"skill_name": "skill_3"}, tool_context=ctx)
current_state = ctx.state.__setitem__.call_args[0][1]

assert current_state == ["skill_2", "skill_3"]

readonly_context.state.get.return_value = list(current_state)
tools = await toolset.get_tools(readonly_context=readonly_context)
tool_names = {t.name for t in tools}
assert "tool_0" not in tool_names
assert "tool_1" not in tool_names # evicted!
assert "tool_2" in tool_names
assert "tool_3" in tool_names