mirror of
https://github.com/mims-harvard/ToolUniverse.git
synced 2026-09-19 07:31:47 +08:00
fix(deps): declare websockets and urllib3, and audit module-scope imports
Three files in the core package import a module at module scope that no
declared dependency provides:
mcp_client_tool.py:13 import websockets
uspto_tool.py:6 from urllib3.util.retry import Retry
euhealth/euhealth_live.py:42 import urllib3
Both worked only because something else happened to install them.
`websockets` arrived through fastmcp's `fastmcp-slim[server]` extra, so an
upstream reshuffle would have broken the websocket transport with no local
change. `urllib3` rode in on requests, which is a hard dependency rather than
an extra, so it is safe today, but the import is still a direct one.
This is the pattern that let `PIL` stay invisible in the USPTO downloader
until issue #521 -- the failure only shows up at call time, in a module the
lazy registry loads without complaint.
Both are now declared. `urllib3>=1.26` matches the floor requests already
imposes, so it adds no new constraint. `websockets>=13.0` sits below the
floor fastmcp-slim[server] sets, so it does not constrain resolution either,
and it deliberately carries no upper bound: the google-genai cap is what
pushes downstream resolvers onto stale tooluniverse releases (issue #526),
and that stays inside its own extra. `uv lock` moves no package version; the
lockfile gains only the two declarations.
`test_core_module_scope_imports_come_from_declared_distributions` now walks
the core package, resolves every module-scope third-party import through
`packages_distributions()`, and fails with file:line when a module has no
declared provider. It stops at `src/tooluniverse/remote/`, where provider
services are deployed separately with their own manifests and their heavy
imports are undeclared on purpose (issue #521, draft PR #523).
Comparing against installed distributions needs real PEP 503 normalization,
or the declared `epam.indigo` never matches the installed `epam-indigo`, so
`_canonical` folds `.`, `-` and `_` the way the spec does. The audit accepts
a module named after its distribution (`fastmcp`), one published under a
different name (`pyyaml` -> `yaml`), and a metapackage that publishes neither
(`fastmcp` -> `fastmcp-slim`).
Verified the audit fails with all three file:line locations when the two
declarations are removed again.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01A2xx4ELgtuMxiDema4LaEh
This commit is contained in:
@@ -40,6 +40,8 @@ dependencies = [
|
||||
"huggingface_hub>=0.34.0",
|
||||
"jsonpath-ng>=1.6.0",
|
||||
"rcsb-api>=1.4.0",
|
||||
"websockets>=13.0", # Module-scope import in mcp_client_tool.py
|
||||
"urllib3>=1.26", # Module-scope import in uspto_tool.py and euhealth_live.py
|
||||
"scipy>=1.7.0",
|
||||
"pandas>=2.2.3",
|
||||
"openpyxl>=3.1.0", # Read .xlsx bulk downloads (CellMarker, dataset tools)
|
||||
|
||||
@@ -29,6 +29,18 @@ dependencies = [
|
||||
"huggingface_hub>=0.34.0",
|
||||
"jsonpath-ng>=1.6.0",
|
||||
"rcsb-api>=1.4.0",
|
||||
# Imported at module scope by mcp_client_tool.py for the websocket
|
||||
# transport. It reached installs only as a transitive of fastmcp's
|
||||
# `fastmcp-slim[server]` extra, so a reshuffle upstream would have broken
|
||||
# the client with no local change. No upper bound on purpose: the
|
||||
# google-genai cap is what pushes downstream resolvers onto stale
|
||||
# tooluniverse releases (issue #526), and that stays inside its extra.
|
||||
"websockets>=13.0",
|
||||
# `from urllib3.util.retry import Retry` in uspto_tool.py and
|
||||
# `urllib3.disable_warnings` in euhealth/euhealth_live.py. requests already
|
||||
# requires urllib3>=1.26,<3, so this adds no new constraint; it just stops
|
||||
# a direct import from depending on another package's dependency.
|
||||
"urllib3>=1.26",
|
||||
# Imported by dose_response, survival, nca, drug_synergy, metaboanalyst,
|
||||
# timer and two remote tools. It was previously reaching installs only as a
|
||||
# transitive of that `fitz` placeholder (which required nibabel, nipype,
|
||||
|
||||
@@ -22,8 +22,10 @@ the root one, and keep every version marker moving together so a fix actually
|
||||
reaches PyPI.
|
||||
"""
|
||||
|
||||
import ast
|
||||
import json
|
||||
import re
|
||||
import sys
|
||||
from importlib.metadata import packages_distributions
|
||||
from pathlib import Path
|
||||
|
||||
@@ -47,6 +49,13 @@ FORBIDDEN_DISTRIBUTIONS = {
|
||||
"fitz": "pymupdf",
|
||||
}
|
||||
|
||||
# `src/tooluniverse/remote/` holds provider-side services that are deployed
|
||||
# separately, each with its own dependency manifest. Their heavy imports (torch,
|
||||
# scanpy, tensorflow, easyocr, ...) are deliberately absent from the SDK's
|
||||
# dependency list, so the module-scope import audit below stops at that boundary.
|
||||
# Undeclared imports inside those services are tracked in issue #521 instead.
|
||||
SEPARATELY_DEPLOYED = "remote"
|
||||
|
||||
# Dependencies the bundle declares on purpose that the root list leaves to an
|
||||
# extra. The bundle is a sealed Claude Desktop runtime: the end user cannot
|
||||
# install an extra into it, so anything optional upstream must be unconditional
|
||||
@@ -74,6 +83,17 @@ def _distribution_name(requirement):
|
||||
return name.lower().replace("_", "-")
|
||||
|
||||
|
||||
def _canonical(name):
|
||||
"""PEP 503 name normalization.
|
||||
|
||||
`_distribution_name` leaves dots alone, which is fine while it only ever
|
||||
compares requirement strings with each other. Comparing against
|
||||
`packages_distributions()` needs the real rule, or the declared
|
||||
`epam.indigo` never matches the installed `epam-indigo`.
|
||||
"""
|
||||
return re.sub(r"[-_.]+", "-", _distribution_name(name))
|
||||
|
||||
|
||||
def _names(pyproject_path):
|
||||
return {_distribution_name(r) for r in _load_dependencies(pyproject_path)}
|
||||
|
||||
@@ -247,6 +267,75 @@ def test_markitdown_requirement_is_unconditional():
|
||||
)
|
||||
|
||||
|
||||
def _module_scope_imports(path):
|
||||
"""Top-level import statements only.
|
||||
|
||||
Imports nested in a try/except or an `if` are already guarded by the module
|
||||
itself and are not the pattern this audit is looking for.
|
||||
"""
|
||||
try:
|
||||
tree = ast.parse(path.read_text(encoding="utf-8", errors="ignore"))
|
||||
except SyntaxError: # pragma: no cover - generated sources are valid
|
||||
return
|
||||
for node in tree.body:
|
||||
if isinstance(node, ast.Import):
|
||||
for alias in node.names:
|
||||
yield alias.name.split(".")[0], node.lineno
|
||||
elif isinstance(node, ast.ImportFrom) and node.level == 0 and node.module:
|
||||
yield node.module.split(".")[0], node.lineno
|
||||
|
||||
|
||||
def test_core_module_scope_imports_come_from_declared_distributions():
|
||||
"""A module the SDK imports at import time must be a dependency we declare.
|
||||
|
||||
`websockets` and `urllib3` both used to fail this. `websockets` reached
|
||||
installs only through fastmcp's `fastmcp-slim[server]` extra, so an upstream
|
||||
reshuffle would have broken `mcp_client_tool` with no local change;
|
||||
`urllib3` rode in on requests. Relying on another package's dependency is
|
||||
how `PIL` stayed invisible in the USPTO downloader until issue #521.
|
||||
"""
|
||||
declared = {
|
||||
_canonical(requirement) for requirement in _load_dependencies(ROOT_PYPROJECT)
|
||||
}
|
||||
declared |= {
|
||||
_canonical(requirement)
|
||||
for requirements in _load_pyproject(ROOT_PYPROJECT)[
|
||||
"optional-dependencies"
|
||||
].values()
|
||||
for requirement in requirements
|
||||
}
|
||||
providers = packages_distributions()
|
||||
first_party = {"tooluniverse"} | {
|
||||
path.stem for path in (REPO_ROOT / "src").iterdir()
|
||||
}
|
||||
|
||||
offenders = []
|
||||
for path in sorted(SRC_ROOT.rglob("*.py")):
|
||||
if SEPARATELY_DEPLOYED in path.relative_to(SRC_ROOT).parts:
|
||||
continue
|
||||
for module, lineno in _module_scope_imports(path):
|
||||
if module in sys.stdlib_module_names or module in first_party:
|
||||
continue
|
||||
# A distribution may publish a module under its own name (`requests`)
|
||||
# or under another one (`pyyaml` -> `yaml`), and a metapackage
|
||||
# publishes neither itself (`fastmcp` -> `fastmcp-slim`). Accept any
|
||||
# of those spellings.
|
||||
candidates = {_canonical(module)}
|
||||
candidates |= {_canonical(dist) for dist in providers.get(module, [])}
|
||||
if candidates & declared:
|
||||
continue
|
||||
offenders.append(
|
||||
f"{path.relative_to(REPO_ROOT)}:{lineno}: `{module}` "
|
||||
f"(installed from {sorted(providers.get(module, [])) or 'nothing'})"
|
||||
)
|
||||
|
||||
assert not offenders, (
|
||||
"These modules are imported at module scope in the core package but no "
|
||||
"declared dependency provides them, so they only work while some other "
|
||||
"package happens to pull them in:\n" + "\n".join(offenders)
|
||||
)
|
||||
|
||||
|
||||
def test_release_versions_move_together():
|
||||
"""A release must bump the package, the bundle, the manifest, and the lock.
|
||||
|
||||
|
||||
@@ -6273,7 +6273,9 @@ dependencies = [
|
||||
{ name = "scipy" },
|
||||
{ name = "setuptools" },
|
||||
{ name = "sympy" },
|
||||
{ name = "urllib3" },
|
||||
{ name = "uvicorn" },
|
||||
{ name = "websockets" },
|
||||
{ name = "xmltodict" },
|
||||
]
|
||||
|
||||
@@ -6521,7 +6523,9 @@ requires-dist = [
|
||||
{ name = "tiledbsoma", marker = "extra == 'singlecell'", specifier = ">=1.15.3" },
|
||||
{ name = "tooluniverse", extras = ["dev", "docs", "graph", "visualization", "space", "embedding", "ml", "bioinformatics", "openai", "gemini"], marker = "extra == 'all'", editable = "." },
|
||||
{ name = "tooluniverse", extras = ["openai", "gemini"], marker = "extra == 'dev'", editable = "." },
|
||||
{ name = "urllib3", specifier = ">=1.26" },
|
||||
{ name = "uvicorn", specifier = ">=0.36.0" },
|
||||
{ name = "websockets", specifier = ">=13.0" },
|
||||
{ name = "werkzeug", marker = "extra == 'graph'", specifier = ">=2.0.0" },
|
||||
{ name = "xmltodict", specifier = ">=1.0.0" },
|
||||
]
|
||||
|
||||
Reference in New Issue
Block a user