Repository navigation
Conversation
The downloader silently stopped working after NCBI retired its anonymous FTP endpoint in 2024. _extract_ftp_links only matched ftp:// URLs, so every E-utils XML response returning https://ftp.ncbi.nlm.nih.gov/... was dropped, producing spurious "no downloadable SOFT file exposed" errors for valid accessions. The URL extractor now accepts both schemes and normalises everything to https://. Two independent bugs blocked the package from importing at all when no config.json existed: - geo_downloader._load_config() ran at module scope and raised FileNotFoundError if no config was found, then sys.exit(1) if email was missing. pytest, mypy, and any `python -c "import geomcp"` died. - main.setup_environment() tried to copy a config_template.json that does not exist in the package, taking sys.exit(1) on first run. Config is now resolved lazily with four layers (hardcoded defaults, config.json, GEOMCP_* env vars, configure() runtime overrides). The legacy ~/.geo-mcp/config.json location remains a fallback so existing Claude Desktop installs keep working. CLI flags (--email, --api-key, --download-dir, etc) now actually apply via geo_downloader.configure() instead of being parsed and ignored. Other correctness fixes: - E-utilities calls moved from blocking requests to aiohttp so the stdio event loop is not stalled during downloads. - asyncio.Semaphore moved off module scope (was bound to the first loop that happened to import us) to a per-loop cache. - Default download directory is XDG ($XDG_DATA_HOME/geomcp/downloads) instead of a package-relative ./downloads that resolved inside site-packages. - main.py no longer os.chdir()s the process or prepends a fictional .venv/bin to PATH. Backward compatibility shims preserved: _extract_ftp_links is an alias for _extract_download_urls, _load_config() returns the new lazy config, setup_environment() is a no-op, --retmax is still accepted (hidden). Adds a tests/ package with 15 regression tests covering the import-time crash, URL scheme handling for both ftp:// and https://, the GSE vs GSM SOFT path convention, configure() override precedence, XDG defaults, legacy config fallback, and the backward-compat shims.
Phase 1 tooling pass for GEOmcp: - Replace the broken test.yml (which invoked pytest against a non-existent geo_mcp_server/test/) and the duplicate python-publish.yml with a one-line ci.yml that delegates to MCPmed/.github's reusable-python-ci.yml (ruff, black, pytest on 3.10/3.11/3.12, codecov upload). Release workflows (pypi-publish, dev-release) kept as-is for now; migration to Trusted Publisher is a follow-up. - Add .pre-commit-config.yaml with ruff, ruff-format, black, and the standard pre-commit-hooks suite. - Add CHANGELOG.md in Keep a Changelog format covering the Tier A download-pipeline fixes and this tooling pass. - Add CONTRIBUTING.md scoped to GEOmcp specifics; defers to the organization-wide contribution guide in MCPmed/.github. - Align license declarations: pyproject.toml classifier and README.md badge now match the BSD-3-Clause LICENSE file (they previously claimed MIT). - Clean up pre-existing lint so CI is green on first run: unused imports (F401) and an unused CallToolRequest assignment (F841) in mcp_server.py; E401 in mcp_http_server.py; black reformat of __init__.py, geo_profiles.py, mcp_http_server.py, mcp_server.py. No behavior changes in those files. - Remove stale .github/WORKFLOWS.md documenting workflows that do not exist as named.
- migrate geo_profiles esearch/esummary to aiohttp and share geo_downloader._config() so GEOMCP_* env vars, configure() overrides, and the XDG config.json finally apply to the search path. Stops the MCP stdio event loop from blocking on every tool call. - drive __version__ and --version from importlib.metadata; remove the hardcoded 0.1.1/0.1.2 constants that drifted from hatch-vcs. - repoint pytest.ini testpaths at tests/ (the old geo_mcp_server/test path no longer exists; pytest was silently collecting zero tests). - drop dead FTP-URL helpers from geo_profiles (download_geo_data, list_downloaded_datasets, get_download_stats); superseded by the async equivalents in geo_downloader. - bump classifier to Beta, refresh README (clone dir, XDG config path, HTTP port 8001, remove stale python main.py --mode stdio examples, soften the dev warning), and finalize CHANGELOG for 0.1.2.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three categories of bugs kept this server from being usable in practice:
@server.call_tool()meantevery call routed to
cleanup_downloads_tooland blew up on anarg-count mismatch.
The URL regex only matched
ftp://.config.jsonexisted —broke pytest, mypy, and any non-server consumer.
This PR fixes all three, migrates the rest of the I/O path to non-blocking
aiohttp, lands the test infra to keep them fixed, and tags 0.1.2.Commits
359c43bfix(downloader): accept https:// URLs from NCBI and lazy-load config65c49b9chore: adopt org-wide tooling baselinee44c7b1release: prep 0.1.2b895362fix(mcp): route every tool call through one dispatcher+1601 / -1627across 20 files.What this fixes
Ordered by user-visible severity, not by file.
Show-stoppers
Every MCP tool call routed to
cleanup_downloads_tooland crashedwith "takes 1 positional argument but 2 were given".
GEOMCPServer._setup_toolscalled@self.server.call_tool()once pertool with a one-argument handler. The MCP SDK registers one handler
per
CallToolRequest— each decorator call replaced the previous one,so only the last-defined function survived — and invokes it as
handler(name, arguments), which the single-arg handlers refused.Replaced the 12 per-tool registrations with a single dispatcher that
delegates to
handle_call_tool(name, arguments). Pinned bytests/test_mcp_server.py(3 tests including a closure-walkingsignature check).
Downloads broken for every accession since mid-2024. NCBI retired
the anonymous FTP endpoint; E-utilities XML now returns
https://ftp.ncbi.nlm.nih.gov/...URLs. The old regex only matchedftp://and silently dropped every response, surfacing as"no downloadable SOFT file exposed". URL extraction now accepts both
schemes and normalises to HTTPS. Pinned by 8 of the
tests/test_downloader.pycases (HTTPS, FTP, GSE-vs-GSM pathconstruction, deduplication, empty-XML).
Package could not be imported without a pre-existing
config.json.geo_downloaderran_load_config()at module scope and calledsys.exit(1)ifemailwas missing. Broke pytest, mypy, and everyconsumer that merely imported the package. Config is now lazy through
_config()with four precedence layers: runtimeconfigure()overrides →
GEOMCP_*env vars →config.json(at$CONFIG_PATH,$XDG_CONFIG_HOME/geomcp/config.json, or legacy~/.geo-mcp/config.json) → built-in defaults.Stdio-loop blocking
geo_profileswas still sync. The downloader migrated toaiohttpin an earlier round, but everysearch_*tool was stillusing blocking
requests+time.sleep(0.1)inside the MCP stdioloop. Migrated to
aiohttp+asyncio.sleep, dropped the duplicateload_config(), now sharesgeo_downloader._config()and_require_email(). Every publicsearch_*is now a coroutine. Toolcall-sites in
mcp_server.pyupdated toawait.asyncio.Semaphorebound to the wrong loop. The module-scopesemaphore captured whichever loop happened to be live at import time
and then raised "attached to a different loop" under MCP stdio. Now
created per-loop in
_get_semaphore().Bootstrap / cleanup
First-run bootstrap was broken.
main.setup_environment()triedto copy a
config_template.jsonthat doesn't exist in the package,then
sys.exit(1)'d. Symbol kept as a no-op shim for backward compat.main.pyno longeros.chdir's the process into the installdirectory or prepends a fictional
.venv/bintoPATH.pytest.initestpathspointed atgeo_mcp_server/test, adirectory that no longer exists. pytest was silently collecting zero
tests. Repointed to
tests/. Without this fix the entire test suitebelow would have been collected-but-not-run on CI.
README usage examples referenced the long-gone
geo_mcp_server/layout, the pre-CLI
python main.py --mode stdioinvocation, port8000 (the HTTP server runs on 8001), and the wrong clone dir
(
GEO_MCPvsGEOmcp). All fixed.What this changes
Config surface
configure(**overrides)runtime API. The CLI flags (--email,--api-key,--download-dir,--max-file-size-mb,--max-total-downloads-mb,--max-concurrent-downloads,--download-timeout-seconds,--allowed-download-paths) now actuallyapply through this path.
GEOMCP_EMAIL,GEOMCP_API_KEY,GEOMCP_BASE_URL,GEOMCP_DOWNLOAD_DIRenv vars as a layer betweenconfig.jsonandconfigure()overrides.geomcp.__version__andgeo-mcp --versionnow read from installedpackage metadata via
importlib.metadata, so they stay in sync withthe wheel/tag instead of drifting from a hand-maintained constant.
Tooling / packaging
.github/workflows/ci.ymldelegating to the org-wide reusable PythonCI workflow (Python 3.10/3.11/3.12, ruff, black, pytest, coverage).
.pre-commit-config.yaml(trailing whitespace, EOF fixer, ruff,ruff-format, black 24.10.0).
CHANGELOG.md,CONTRIBUTING.md, MIT/BSD license alignment.Development Status :: 4 - Beta.F401,F841,E401) and black drift soCI is green on the first run.
What this removes
python-publish.ymloverlapped withpypi-publish.yml; both fired onrelease: publishedand woulddouble-upload).
test.ymlworkflow that ran pytest against a non-existent dir..github/WORKFLOWS.mdthat documented a workflow layout therepo never actually had.
geo_profiles: legacy-FTP-URLdownload_geo_data,list_downloaded_datasets,get_download_stats— superseded by theirasync equivalents in
geo_downloader. Nothing imported them.Breaking changes
For the uncommon case only. Default
download_diris now$XDG_DATA_HOME/geomcp/downloadsinstead of./downloadsresolvedrelative to the installed package directory. Users with an existing
~/.geo-mcp/config.jsonare unaffected — that location is still honoredas a fallback, and
geo-mcp --initwrites there if it already exists sono orphaning.
The public Python API is unchanged in shape; the
search_*functions ingeomcp.geo_profilesare now coroutines, but they were never documentedas sync-callable from external code.
Verification
ruff check .— cleanblack --check .— 10 files unchangedpytest -q— 18 passed in 0.36sgeo-mcp --version— renders fromimportlib.metadata(
0.1.dev15+ge44c7b1e5in the dev install; will become0.1.2after the tag)
inspect.iscoroutinefunction(geomcp.geo_profiles.search_geo)—Trueget_tool_definitions()has a branch in
handle_call_tool(verified by both theregression test and an out-of-band parity check)
~/.geo-mcp/config.jsonstill read by_config()~/.venvs/geomcp— tool calls dispatch correctlydownload_geo_dataagainstGSE10072, thenget_download_statusandcleanup_downloads_tool.Couldn't run from the venv used for prep; do this once the PR is
merged and the published wheel is installed.
Test inventory
tests/test_downloader.py— 15 cases:_require_emailraises only on use, not on importftp://accepted,https://accepted, directSOFT-file URL passed through, GSM-vs-GSE soft path construction,
deduplication, empty-XML returns
[]_extract_ftp_links,_load_config)configure()overrides win,Noneoverrides ignoreddownload_diris XDG (not site-packages)~/.geo-mcp/config.jsonstill honoredsetup_environment()no-op shim still callabletests/test_mcp_server.py— 3 cases:get_tool_definitionsdispatches cleanly throughhandle_call_tool(with stubbed backends so no network)ValueErrorCallToolRequesthandler accepts two positional arguments(closure-walks through the SDK's wrapper to find the user fn)
Reviewer notes (non-obvious choices)
_DISPATCHdict at module scope but reverted it: a dict capturesfunction references at import time, which makes the
monkeypatch-based tests ineffective. The bigif/elifdoesattribute-lookups at call time, so stubbing
geomcp.geo_profiles.search_geoactually changes what runs.handle_list_toolsexists at module scope even though_register()already wires@server.list_tools(). Reason:mcp_http_server.pyimportshandle_list_toolsdirectly and callsit. Removing the module-level symbol would break the HTTP gateway.
mcp_http_server.pystill has anasync def main()with a staleos.chdir. It is only reachable viapython -m geomcp.mcp_http_server; the user-facinggeo-mcp --httppath goesthrough
main.run_http_server()which does not chdir. Left in placerather than expand the diff further; tracked for a follow-up.
vcrpyfor cassette-recorded networktests.
vcrpyis not added to dev deps; this is aspirationalguidance, not enforced. Fine for now since no current test needs it.
Development Status :: 4 - Betais a deliberate step from"Alpha". The README "not production-ready" warning was softened to a
Beta notice. We can move to "5 - Production/Stable" after a couple of
bug-free release cycles.
2026-04-17(when prep work landed). If thetag goes out on a different day, bump that line in the same commit
as the tag — no other state to migrate.