feat(agent_registry): support published_skills accessor and project alias - #7174
ArulJerald wants to merge 1 commit into
Conversation
…lias Allow AgentRegistry(project=...) as an alias for project_id to align with Google Cloud Console snippets, and introduce a published_skills accessor that fetches and loads published skills into Skill objects. Closes google#7137
codebee-aoki
left a comment
codebee-aoki
left a comment
There was a problem hiding this comment.
Thanks @ArulJerald. The API shape matches what #7137 asks for: synchronous, takes the full resource name, returns a Skill that drops into SkillToolset(skills=[...]), and project= is accepted. 72 unit tests pass locally. Unfortunately it does not work against the real service yet.
Blocker: wrong API version. Running the Console snippet verbatim against a real catalog fails with HTTP 404 on https://agentregistry.googleapis.com/v1/projects/.../skills/.... AgentRegistry uses the /v1 base, but skills only exist in v1alpha: the public discovery document for v1 lists agents, aiApplications, bindings, endpoints, mcpServers, operations and services under locations, while v1alpha adds publishers and skills (GCPSkillRegistry already uses v1alpha). With only the base URL switched to v1alpha, the snippet runs end to end in about 3 seconds and the Google-published skill cloud.google.com-google-cloud-networking-observability loads and works in SkillToolset. The unit tests do not catch this because HTTP is mocked and no test asserts the requested URL. Suggest a dedicated v1alpha base for the skill calls (mirroring GCPSkillRegistry, including the mTLS template and the AGENT_REGISTRY_ENDPOINT override) and asserting the URL in the tests.
The remaining points are inline.
| ) | ||
|
|
||
|
|
||
| _SKILL_RESOURCE_NAME_PATTERN = re.compile( |
There was a problem hiding this comment.
[^/]+ accepts .., ? and %. Against the real service, .../skills/.. sends an authorized GET to /v1alpha/projects/<p>/locations/global/ (200), and .../skills/x?alt=media injects the query parameter into the metadata call. Please validate each segment before any request, e.g. with the safe-id rule from #7138 (^[a-z0-9]+(?:[._-][a-z0-9]+)*$, max 256).
| params=params, | ||
| allow_redirects=True, | ||
| ) | ||
| if 300 <= response.status_code < 400 and ( |
There was a problem hiding this comment.
allow_redirects=True already follows redirects, and requests drops Authorization when the host changes. This fallback re-issues the request through AuthorizedSession, which attaches the bearer token to whatever Location says (the new redirect test exercises exactly this with storage.googleapis.com). In the real run the redirect was same-host (/download/v1alpha/...) and this branch was never reached. Please remove it.
| if not default_revision: | ||
| raise ValueError(f"Skill '{name}' does not contain default revision.") | ||
|
|
||
| if default_revision.startswith("http://") or default_revision.startswith( |
There was a problem hiding this comment.
The API returns defaultRevision as projects/.... Sending an authorized request to an arbitrary absolute URL taken from a response body is unnecessary; keeping only the projects/ form (as GCPSkillRegistry does) is simpler and safer.
| project: Optional alias for project_id. | ||
| """ | ||
| self.project_id = project_id | ||
| self.project_id = project_id or project |
There was a problem hiding this comment.
If both project_id and project are given and differ, this silently picks project_id. Please raise ValueError instead.
| return self._registry._fetch_published_skill_sync(name) | ||
|
|
||
|
|
||
| _PublishedSkillsAccessor = PublishedSkills |
There was a problem hiding this comment.
_PublishedSkillsAccessor is unused.
3 participants
Link to Issue or Description of Change
1. Link to an existing issue (if applicable):
Problem:
Google Cloud Console code snippets for Agent Registry instruct users to initialize and fetch published skills using:
In ADK,
AgentRegistry.__init__previously only acceptedproject_id, resulting in aTypeError: unexpected keyword argument 'project'. Additionally,AgentRegistrylacked apublished_skillsaccessor and had no method to fetch, download, and deserialize published skills from the registry intoSkillmodels for use withSkillToolset.Solution:
project: str | None = Noneparameter toAgentRegistry.__init__as an alias forproject_id, along with a@property def project(self) -> str | None.PublishedSkillsaccessor exposed viaregistry.published_skillswith synchronousget(name: str) -> Skill(and added a top-levelget_published_skillconvenience method).projects/{project}/locations/{location}/skills/{skill_id}.AuthorizedSession(handling 302/307 redirects to media URLs).google.adk.skills.models.Skillinstance ready to pass toSkillToolset(skills=[...]).Testing Plan
Unit Tests:
Summary of passing
pytestresults:uv run pytest tests/unittests/integrations/agent_registry/test_agent_registry.py: 72 passed in 1.96sAgentRegistry(project="...", location="...")andproject_idprecedence.registry.published_skills.get(name=...)with mock metadata and media responses.registry.get_published_skill(...).Skilldirectly toSkillToolset(skills=[skill]).Manual End-to-End (E2E) Tests:
N/A - verified via unit tests with mocked API endpoints and
AuthorizedSessionbehavior.Checklist