Skip to content

Release async tool contexts when callable validation fails - #1978

Open
sylvesterkaczmarek wants to merge 1 commit into
anthropics:mainfrom
sylvesterkaczmarek:fix/async-tool-context-validation-cleanup-20261005
Open

sylvesterkaczmarek wants to merge 1 commit into
anthropics:mainfrom
sylvesterkaczmarek:fix/async-tool-context-validation-cleanup-20261005

Conversation

@sylvesterkaczmarek

Copy link
Copy Markdown

Fixes #1977.

If validation of the callable yielded by an async context manager fails, call and await __aexit__ with the active exception before re-raising. This mirrors the existing synchronous construction-failure path. Successful lazy initialization and normal tool cleanup remain unchanged.

The new cases use real async context managers and real Pydantic schema failures. They verify that cleanup finishes before the error is returned, receives the original exception, cannot suppress the setup failure, and is not repeated by later cleanup. The synchronous failure case remains a passing control.

Validation

  • Unchanged implementation: two async regressions fail; the synchronous control passes.
  • Complete function-tool tests: 27 passed on Python 3.10.16 and 3.14.7 under Pydantic v2.
  • The script's MCP v2 follow-up: 39 passed on each Python version.
  • Pydantic v1 run: 27 skipped, as the existing function-tool tests require v2.
  • Repository formatting script run; ./scripts/lint passed with all development extras installed, including Ruff, dependency-cap checks, strict Pyright and the import smoke test.
  • git diff --check passed.
UV_PYTHON=3.10 TEST_API_BASE_URL=http://127.0.0.1:9 PYTEST_ADDOPTS='-n 2' \
  ./scripts/test tests/lib/tools/test_functions.py -n 2
UV_PYTHON='>=3.14.0' TEST_API_BASE_URL=http://127.0.0.1:9 PYTEST_ADDOPTS='-n 2' \
  ./scripts/test tests/lib/tools/test_functions.py -n 2
UV_PYTHON=3.10 UV_NO_SYNC=1 ./scripts/lint

Tests ran on macOS and use local fixtures rather than the configured API URL. The full repository suite and live sessions were not run. No public API, dependency, lockfile or workflow changes.

Signed-off-by: Sylvester Kaczmarek <16242628+sylvesterkaczmarek@users.noreply.github.com>
@sylvesterkaczmarek
sylvesterkaczmarek requested a review from a team as a code owner October 5, 2026 10:57

@sigley sigley left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Validated exact head 05f9dedb independently. The async context-manager path enters the resource before pydantic.validate_call(inner); on validation failure this head now awaits __aexit__ with the active exception before re-raising, matching the synchronous cleanup behavior and preventing the entered resource from being stranded. The regressions also cover attempted suppression and verify cleanup is not repeated later. The branch merges cleanly with current main, the complete function-tool test file passes 27/27, and compile/diff checks are clean. This is the earlier, broader-tested branch for #1977, so I would consolidate here rather than duplicate the same production fix. I do not see a blocking correctness issue.

This branch has not been deployed

No deployments
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.

Async context-manager tools leak entered resources when callable validation fails

2 participants