Skip to content

feat(skills): let SkillToolset pin registry skills so they appear in list_skills / the catalog without a search_skills turn #7130

Description

@codebee-aoki

🔴 Required Information

Is your feature request related to a specific problem?

When SkillToolset is backed by a SkillRegistry (e.g. GCPSkillRegistry / Agent Registry), skills that live in the registry are never part of the L1 catalog the model sees. The model can only reach them through search_skillsload_skill, so a registry skill always costs at least one more model turn (plus one registry round-trip) than an equivalent local skill, even when the developer knows up front exactly which registry skills the agent should use.

Current behavior on main (ac0133a):

  • list_skills and the <available_skills> catalog are built from SkillToolset._list_skills(), which only returns the skills passed via skills=[...] (src/google/adk/tools/skill_toolset.py:1594-1596). The registry is never consulted for the catalog.
  • The registry is only consulted from _get_or_fetch_skill() (skill_toolset.py:1582,1592), i.e. after the model has already named the skill. The system instruction tells the model to fall back to search_skills when local skills are not sufficient (skill_toolset.py:1669-1675).
  • The new SkillDiscoveryMode.EAGER (feat(skills): add include_list_skills to inject the L1 catalog without list_skills #7092 / feat(skills): add SkillDiscoveryMode to skip the discovery turn #7093) removes the discovery turn for local skills, but its docstring explicitly leaves registry skills out: "Registry skills are unaffected: they are still reachable only through search_skills." So after that change the gap is: local skills = 0 discovery turns, registry skills = always ≥ 1.
  • As a secondary cost, a registry skill the model has activated is re-downloaded on every later invocation: session state only keeps the activated skill names (_adk_activated_skill_<agent>), and _fetched_skill_cache is keyed by invocation_id (skill_toolset.py:1564), so _resolve_additional_tools_from_state() calls registry.get_skill() again each turn.

Concretely, for a fixed set of skills that happen to be published in Agent Registry instead of shipped with the agent:

local skill registry skill
turns before load_skill (LAZY) 1 (list_skills) 2 (list_skillssearch_skills)
turns before load_skill (EAGER) 0 1 (search_skills)
registry round-trips per later turn 0 1 per activated skill

The extra turn also adds the search_skills tool declaration, the search result payload and the model's reasoning about it to the context on every conversation, which is the token waste we want to avoid. Our use case is the common one where the skill content is managed centrally in the registry (its update cycle is decoupled from the agent's deploy cycle), but the set of skills an agent uses is decided at build time, exactly like local skills.

Describe the Solution You'd Like

Let SkillToolset pin a known set of registry skills so they behave like local skills once fetched:

toolset = SkillToolset(
    skills=load_skills_from_dir("./skills"),           # local, as today
    registry=GCPSkillRegistry(project_id=..., location=...),
    registry_skills=["shared-skill-a", "shared-skill-b"],  # NEW
    discovery_mode=SkillDiscoveryMode.EAGER,
)

Proposed semantics:

  • registry_skills: list[str] | None = None (keyword-only, default None, fully backward compatible).
  • The named skills are fetched lazily, once, on the first get_tools() / process_llm_request() call for the toolset (both are already async, and the agent module itself is usually imported synchronously, sometimes inside a running event loop such as adk run / adk web, so asyncio.run at import time is not an option). Fetches for the whole list run concurrently and are guarded by a lock so concurrent first requests only fetch once.
  • After the fetch the skills are stored alongside the local ones, so they appear in list_skills, in the EAGER catalog, and _get_or_fetch_skill resolves them without touching the registry again for the lifetime of the process. search_skills keeps working for skills that are not pinned.
  • A public async def prefetch() so callers that do have a startup hook (e.g. Agent Engine AdkApp.set_up(), which runs synchronously with no loop, or a FastAPI lifespan) can warm the toolset before the first request.
  • Name collisions: a fetched registry skill whose SKILL.md frontmatter name already exists locally is skipped with a warning and the local skill wins. This matches the policy SearchSkillsTool already applies ("Skill naming conflict ... Registry skill is filtered."). Note the skill name comes from the archive's frontmatter, not from the registry resource name, so the check must happen after the fetch.
  • Fetch failures are logged and leave the toolset usable (the search_skills path still works); the next request retries.
  • clone_with_updated_skills() forwards registry_skills, so the pinned set survives cloning.

Impact on your work

We run agents on Agent Engine that consume skills published in Agent Registry. Every conversation pays a search_skills turn before the skill can be loaded, and every subsequent turn re-downloads the skill archive. We currently work around this with a SkillToolset subclass that does the lazy prefetch described above, but it has to reach into private attributes (_skills, _registry) and is dropped by clone_with_updated_skills(), so we would much rather have it in the library. Not blocking, but it affects latency and token cost on every request.

Willingness to contribute

Yes. Happy to open a PR with the parameter, the lazy prefetch, prefetch(), and unit tests, once the maintainers agree on the shape. (Related: #6908 / #6824GCPSkillRegistry.get_skill() currently fails on the media-download redirect, so the prefetch needs that fix to work end to end against the real Agent Registry.)


🟡 Recommended Information

Describe Alternatives You've Considered

  • Fetch at startup and pass the result via skills=[...]. Works in principle, but get_skill() is async and there is no sync counterpart, and the agent module is imported inside a running event loop by adk run / adk web, so asyncio.run() at module import is not portable.
  • Snapshot the registry skills into the local skills/ directory at deploy time. Removes the runtime dependency entirely, but couples skill updates to the agent's deploy cycle, which is the opposite of why the skills are in a registry.
  • Extend SkillDiscoveryMode.EAGER to include registry skills automatically. Would require a "list all" operation on SkillRegistry, which the interface does not have (only get_skill and search_skills), and would inject an unbounded catalog. An explicit allow-list keeps the catalog bounded and the registry interface unchanged.
  • Cache fetched skills across invocations (key _fetched_skill_cache by skill name instead of invocation_id). Solves the re-download cost but not the discovery turn; it would also be a useful, independent change and could be a separate PR.

Proposed API / Implementation

Sketch of the core (names are placeholders):

class SkillToolset(BaseToolset):
  def __init__(self, ..., registry_skills: list[str] | None = None):
    ...
    self._registry_skills = list(registry_skills or [])
    self._registry_skills_loaded = False
    self._registry_skills_lock = asyncio.Lock()

  async def prefetch(self) -> None:
    if self._registry_skills_loaded or not self._registry_skills:
      return
    async with self._registry_skills_lock:
      if self._registry_skills_loaded:
        return
      missing = [n for n in self._registry_skills if n not in self._skills]
      fetched = await asyncio.gather(
          *(self._registry.get_skill(name=n) for n in missing)
      )
      for skill in fetched:
        if skill.name in self._skills:
          logger.warning("Registry skill %r collides with a local skill; keeping local", skill.name)
          continue
        self._skills[skill.name] = skill
      self._registry_skills_loaded = True

  async def get_tools(self, readonly_context=None):
    await self.prefetch()
    ...

  async def process_llm_request(self, *, tool_context, llm_request):
    await self.prefetch()
    ...

Additional Context

Activity

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

Metadata

Metadata

Assignees

Labels

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

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions