Skip to content

fix(flows): resolve toolset credentials before resuming a paused call - #7941

Open
AtulJoshi1206 wants to merge 1 commit into
google:mainfrom
AtulJoshi1206:fix/resume-toolset-credentials
Open

AtulJoshi1206 wants to merge 1 commit into
google:mainfrom
AtulJoshi1206:fix/resume-toolset-credentials

Conversation

@AtulJoshi1206

Copy link
Copy Markdown
Contributor

Please ensure you have read the contribution guide before creating a pull request.

Link to Issue or Description of Change

No existing issue; described below following the bug-report structure.

Describe the Bug:

When a paused tool call is resumed (an approved confirmation, or a tool-level auth answer), the request_confirmation and auth request processors list the agent's tools themselves so they can re-run the call. Both run before the toolset_auth processor, and that processor is the only place that puts an authenticated toolset's credential for the invocation into credential_by_key.

So an McpToolset with an auth_scheme, or any toolset using get_auth_config(), lists its tools without its credential. That breaks the contract in BaseToolset.get_auth_config and in the toolset guide ("ADK resolves the credential before it calls get_tools"). It has only worked while the toolset still cached the paused invocation's listing.

Impact:

Human-in-the-loop approval on an authenticated MCP server fails whenever that cache doesn't hold the paused invocation's listing. That happens:

  • after any other invocation lists the same toolset, because the cache holds one entry;
  • when the approval reaches a restarted process or another instance.

So it happens in pretty much any real deployment. On a single-session dev setup it works, which is probably why nobody noticed.

Observed Behavior:

  • Approving the call: the toolset connects to the server without the bearer token, the server refuses it, and the toolset is dropped. The invocation then fails with ValueError: Tool 'send_email' is not registered. The email is never sent, and sending the approval again fails the same way.
  • Answering a tool-level credential request: the resumed call gets Invoking export_report() failed as no tool with that name is available. The tools you can call are: none., and the tool never runs.

Expected Behavior:

The resumed call lists the toolset with the user's credential and runs, the same as on the first turn.

Solution:

I moved the credential loop out of resolve_toolset_auth into resolve_toolset_credentials. It fills credential_by_key with whatever is available and returns the toolsets that still need auth, without asking the user for anything.

  • Both resume processors call it right before they list tools.
  • resolve_toolset_auth behaves exactly as before, so nothing changes on the normal model-request path.
  • Auth responses are already exchanged when they're stored, so resolving twice in a resume step never redeems a code twice.

I looked at the obvious alternatives:

  • Reordering the processors doesn't work. request_confirmation has to run before contents, and the auth processor has to store the response before toolset auth reads it.
  • Asking for auth from inside the resume would lose the confirmation.

Testing Plan

Unit Tests:

  • I have added or updated unit tests for my change.
  • All unit tests pass locally.

New tests:

  • TestMcpToolsetCredentialOnResume runs a real in-process MCP server through Runner and McpToolset; the server refuses connections without the bearer token. It covers an approval served by another process, and an approval after another session's turn.
  • TestToolsetCredentialOnAuthResume covers a toolset that uses the documented get_auth_config pattern, whose tool asks for its own credential.
  • test_resolving_credentials_requests_nothing covers the new helper.

On main, all 4 fail with the errors above. With the fix, all 4 pass.

pytest tests/unittests/tools tests/unittests/flows tests/unittests/runners tests/unittests/auth \
  tests/unittests/test_runners.py tests/unittests/test_import_loading.py -n 8
main: 4 failed (the new tests), 5419 passed, 2 skipped, 3 xfailed
fix:  5423 passed, 2 skipped, 3 xfailed

pyink and isort are clean, and mypy reports nothing new.

Manual End-to-End (E2E) Tests:

Not run against a live MCP server. The runner tests drive the full flow with an in-process server.

Checklist

  • I have read the CONTRIBUTING.md document.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have added tests that prove my fix is effective or that my feature works.
  • New and existing unit tests pass locally with my changes.
  • I have manually tested my changes end-to-end.
  • Any dependent changes have been merged and published in downstream modules. (none)

Additional context

#7909 fixes a different MCP auth issue (credential_key not reaching the tools). The two are independent; both just append to test_mcp_toolset_auth.py.

🤖 Generated with Claude Code

The request_confirmation and auth request processors list the agent's tools
to re-run a call that was waiting on the user. They run before the
toolset_auth processor, which is what puts each authenticated toolset's
credential for this invocation into credential_by_key. So when they list an
McpToolset that has an auth_scheme, the toolset finds no credential and
connects without one.

That only worked while the toolset still cached the paused invocation's tool
listing. Any other invocation listing the same toolset replaces that single
cached entry, and a restarted process or another instance has no entry at
all. Then an authenticated server refuses the anonymous connection, the
toolset is dropped, and an approved call fails the invocation with
"Tool '<name>' is not registered". A call resumed after tool-level auth is
answered with "no tool with that name is available" instead of being run.

Factor the credential resolution out of resolve_toolset_auth into
resolve_toolset_credentials, which resolves what is available without asking
the user for anything. Call it from both resume processors before they list
tools. Nothing changes for the model request path.
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