Skip to content

Add Toolset Authentication per issue #1227 - 4 - #2138

Merged
zyantw merged 8 commits into
mainfrom
zyantw-patch-5
Oct 8, 2026
Merged

zyantw merged 8 commits into
mainfrom
zyantw-patch-5

Conversation

@zyantw

@zyantw zyantw commented Aug 17, 2026 •

Copy link
Copy Markdown
Collaborator

Added documentation for the new Toolset-level authentication feature and improved the visual rendering of our authentication flow diagrams.


Technical Report: Toolset Authentication Documentation Verification


1. Verification of Import Statements

Snippet Under Review

from google.adk.auth import AuthConfig
from google.adk.tools.base_toolset import BaseToolset

2. Method Signature and Subclassing

Snippet Under Review

class MyAuthenticatedToolset(BaseToolset):

  def get_auth_config(self) -> AuthConfig | None:
    return AuthConfig(...)

3. Pre-Execution Authentication Resolution

Draft Statement Under Review

"Under the hood, the BaseLlmFlow automatically checks your BaseToolset for authentication requirements before it even lists or executes any tools. It does this by checking the toolset's get_auth_config() method."


4. Pausing Execution and Emitting adk_request_credential

Draft Statement Under Review

"1. Pause execution: It safely halts the current flow. 2. Request credentials: It issues an adk_request_credential event to the client..."

Added documentation for the new Toolset-level authentication feature and improved the visual rendering of our authentication flow diagrams.
@netlify

netlify Bot commented Aug 17, 2026 •

Copy link
Copy Markdown

✅ Deploy Preview for adk-docs-preview ready!

Name Link
🔨 Latest commit 435bafe
🔍 Latest deploy log https://app.netlify.com/projects/adk-docs-preview/deploys/6ac8258f5993480008ca7fc7
😎 Deploy Preview https://deploy-preview-2138--adk-docs-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

@zyantw
zyantw requested a review from joefernandez August 17, 2026 21:01
@zyantw zyantw self-assigned this Aug 17, 2026
Comment thread docs/tools-custom/authentication.md Outdated

@joefernandez joefernandez left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Content review report

  • Review Score: 95
  • Overall assessment: 🟡 Medium Risk
  • Summary reasoning: The prose describing toolset-level auth is accurate against _toolset_auth.resolve_toolset_auth and BaseToolset.get_auth_config. The one substantive problem is the new sequence diagram: it is headed "Toolset authentication flow" but draws the per-tool request_credential handshake with BigQuery participants, which misrepresents a shipped capability. The remaining items are a malformed link, a non-runnable code sample and a missing version note.
  • Recommended action: Approve with these specific changes. The diagram should be redrawn (or dropped) so it does not imply BigQueryToolset uses toolset-level auth — it does not override get_auth_config() (bigquery_toolset.py) — and the sample should be made runnable. No legal, privacy, security, pricing or third-party-claim risk in this change.
  • Number of lines changed: M, 55
  • Number of files changed: 1

🟡 Medium risk items

  • The #### Toolset authentication flow mermaid diagram at lines 434-457 depicts the per-tool tool_context.request_credential handshake (Tool-->>Agent: Request Credentials, Tool->>Google: Run API Call (BigQuery)), not the toolset-level flow the section describes, where the framework resolves auth before get_tools() is called. It is also a near-duplicate of the auth_part1.svg diagram at line 401. Because the participants are named for BigQuery, a reader will reasonably conclude the shipped BigQueryToolset uses toolset-level auth; it does not. Suggested change: redraw with generic participants and the get_auth_config() → adk_request_credential → get_tools() ordering (inline suggestion below), or remove the diagram.

🟢 Low risk items

  • Line 412: the link to "Handle the interactive OAuth/OIDC flow" has a stray trailing ) that renders as literal text, and it is an absolute https://adk.dev URL for a heading on this same page. Use the in-page anchor #handle-the-interactive-oauthoidc-flow-client-side.

  • Lines 419-430: the sample is not runnable. AuthConfig(...) passes a literal Ellipsis where the required auth_scheme field belongs (auth_tool.py#L59), and get_tools() is an @abstractmethod (base_toolset.py#L90), so MyAuthenticatedToolset cannot be instantiated.

  • Line 416: no availability note. get_auth_config() and the framework support that calls it first shipped in google-adk v1.24.0 (commit 381d44ca, 2026-01-29). On earlier releases the base method returns None (base_toolset.py#L264) and nothing calls it, so the feature silently does nothing.

Cosmetic, non-risk: trailing whitespace on lines 407, 444, 446 and 451.

Note: the AuthConfig | None return annotation is fine as written — google-adk requires Python >= 3.10 and ADK itself uses that form (agent_registry.py#L133) — so no from __future__ import annotations is needed.

Comment thread docs/tools-custom/authentication.md Outdated
Comment thread docs/tools-custom/authentication.md Outdated
Comment thread docs/tools-custom/authentication.md
Comment thread docs/tools-custom/authentication.md Outdated
Comment thread docs/tools-custom/authentication.md Outdated
Comment thread docs/tools-custom/authentication.md Outdated
@zyantw
zyantw requested a review from joefernandez October 5, 2026 22:51
Comment thread docs/tools-custom/authentication.md
Comment thread docs/tools-custom/authentication.md
Comment thread docs/tools-custom/authentication.md Outdated
Comment thread docs/tools-custom/authentication.md Outdated
Comment thread docs/tools-custom/authentication.md Outdated
Comment thread docs/tools-custom/authentication.md Outdated
Comment thread docs/tools-custom/authentication.md Outdated

@joefernandez joefernandez left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved with a few comments.

Note that I recommend dropping the new mermaid diagram

@zyantw
zyantw merged commit 57e34aa into main Oct 8, 2026
11 of 12 checks passed
@zyantw
zyantw deleted the zyantw-patch-5 branch October 8, 2026 23:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants