Skip to content

fix(core): allow load_toolset strict success when all params are used#735

Open
hsusul wants to merge 1 commit into
googleapis:mainfrom
hsusul:fix/load-toolset-strict-success-path
Open

fix(core): allow load_toolset strict success when all params are used#735
hsusul wants to merge 1 commit into
googleapis:mainfrom
hsusul:fix/load-toolset-strict-success-path

Conversation

@hsusul

@hsusul hsusul commented Jul 24, 2026

Copy link
Copy Markdown

Summary

  • Fixes ToolboxClient.load_toolset(..., strict=True) incorrectly rejecting loads where every provided auth token / bound parameter is used by every tool.
  • Root cause: used-key accumulation ran only in the non-strict branch, so the final toolset-level unused-requirement check always saw empty overall-used sets under strict=True.
  • Adds async and sync regression coverage for the documented strict success path, while keeping existing strict/non-strict failure tests.

🛠️ Fixes #734

Problem

With a mocked toolset containing a single tool that declares param_P:

await client.load_toolset(
    bound_params={"param_P": "some_value"},
    strict=True,
)

Before: raised

ValueError: Validation failed for toolset 'default': unused bound parameters could not be applied to any tool: param_P.

even though the tool used param_P. The same false failure occurred for fully used auth token getters.

After: loading succeeds, matching the strict docstring (fail only when any tool fails to use all provided params/tokens).

No GitHub issue was required for permissions reasons beyond standard contribution flow; issue #734 was filed to satisfy the PR template's issue-first checklist.

Root cause

In ToolboxClient.load_toolset, per-tool usage was recorded only in the else (non-strict) branch:

if strict:
    validate_unused_requirements(...)  # per-tool
else:
    overall_used_auth_keys.update(...)
    overall_used_bound_params.update(...)

validate_unused_requirements(..., overall_used_*, is_toolset=True)  # always runs

Under strict=True, overall_used_* stayed empty, so the final toolset check treated every provided key as unused.

Minimal implementation

Always update overall_used_auth_keys / overall_used_bound_params, then still run the per-tool strict check. Non-strict semantics are unchanged. Sync client inherits the fix via ToolboxSyncClient.load_toolset.

Tests

  • test_load_toolset_strict_with_fully_used_bound_param_success
  • test_load_toolset_strict_with_fully_used_auth_success
  • test_sync_load_toolset_strict_with_fully_used_bound_param_success
  • Existing strict partial-use and non-strict unused failure tests still pass

Validation

From packages/toolbox-core with pip install -e .[test] (Python 3.12):

  • python -m pytest tests/test_client.py::TestValidation tests/test_sync_client.py::test_sync_load_toolset_strict_with_fully_used_bound_param_success -v9 passed
  • python -m pytest tests/ --ignore=tests/test_e2e.py --ignore=tests/test_sync_e2e.py --ignore=tests/test_e2e_mcp.py --ignore=tests/conformance -q --cov=src/toolbox_core --cov-fail-under=90478 passed, coverage 91.74%
  • black --check . → pass
  • isort --check . → pass
  • MYPYPATH='./src' mypy --install-types --non-interactive -p toolbox_core → pass
  • git diff --check → pass

Live Toolbox / GCP integration tests were not run (mocked unit coverage is sufficient for this client validation bug).

Adapter compatibility

No adapter changes. LangChain / LlamaIndex / ADK call into the same core load_toolset path and inherit the corrected strict behavior.

Non-goals

  • Changing strict vs non-strict semantics beyond restoring the documented success path
  • MCP isError handling, schema conversion, or ADK auth lifecycle (separate candidates)
  • Dependency upgrades or docs-only changes

Test plan

  • Unit regression for strict success (bound params + auth) on async client
  • Unit regression for strict success on sync client
  • Existing strict/non-strict unused validation tests still pass
  • Lint (black, isort) and mypy for toolbox_core
  • Maintainer-triggered CI (tests: run label / /gcbrun if needed for fork PRs)

Accumulate used auth tokens and bound params under strict=True so the
final toolset unused-requirement check no longer rejects valid loads.
@hsusul
hsusul requested a review from a team as a code owner July 24, 2026 21:45
@google-cla

google-cla Bot commented Jul 24, 2026

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

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.

fix(toolbox-core): load_toolset(strict=True) rejects fully used auth tokens and bound params

2 participants