From 27c7c4b5dfb6636ce67ec7726114b5dcf91bf7d4 Mon Sep 17 00:00:00 2001 From: John Anderson Date: Wed, 7 Oct 2026 13:13:11 -0400 Subject: [PATCH 01/16] test: a missing dataset export node crashes platform_dataset_lookup --- tests/test_tools_platform.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/test_tools_platform.py b/tests/test_tools_platform.py index 31fc9a9..e41cc51 100644 --- a/tests/test_tools_platform.py +++ b/tests/test_tools_platform.py @@ -7,6 +7,8 @@ Tests for Platform MCP tools using FastMCP's in-memory testing pattern. """ +import json + from unittest.mock import ANY, MagicMock import httpx @@ -665,6 +667,17 @@ async def test_complete_timeout_delayed(self, succeeded, mangle, value, async_sl assert async_sleeps == [2, 4] self.assert_result(result, started=True, succeeded=succeeded) + async def test_not_found(self): + # The platform returns a null node for an unknown ID, and for an export that + # belongs to someone else. + not_found = self.expect_get_export({"id": self.DATASET_ID}) + not_found.response = json.dumps({"data": {"node": None}}) + + with self.http.expect(not_found): + result = await self.assert_call({"dataset_id": self.DATASET_ID}, error=True) + + assert result.text.startswith(f"Dataset export {self.DATASET_ID} not found") + class TestToolAnnotations: """The behavioural hints hosts show when asking to approve a tool call.""" From 30b5dc2456e63cbdaaf851fbcfb1a435f3dbc337 Mon Sep 17 00:00:00 2001 From: John Anderson Date: Wed, 7 Oct 2026 13:13:25 -0400 Subject: [PATCH 02/16] fix: report a missing dataset export instead of crashing --- stacklet/mcp/platform/graphql.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/stacklet/mcp/platform/graphql.py b/stacklet/mcp/platform/graphql.py index c052450..ac8f5ac 100644 --- a/stacklet/mcp/platform/graphql.py +++ b/stacklet/mcp/platform/graphql.py @@ -225,6 +225,12 @@ async def _get_export(self, dataset_id: str) -> ConnectionExport: # If no errors, data is at least guaranteed guaranteed truthy. fields = cast(dict[str, Any], result.data)["node"] + if fields is None: + raise AnnotatedError( + problem=f"Dataset export {dataset_id} not found", + likely_cause="the ID is wrong, or the export belongs to another user", + next_steps="start a new export with platform_dataset_export", + ) return ConnectionExport(**fields) Q_GET_EXPORT = """ From ba0a6f8ac60950f4708bd22fcc8105c013f80b5a Mon Sep 17 00:00:00 2001 From: John Anderson Date: Wed, 7 Oct 2026 13:16:54 -0400 Subject: [PATCH 03/16] feat: download dataset exports with the caller's credentials --- NEWS.md | 6 ++ README.md | 2 +- stacklet/mcp/platform/dataset_info.md | 6 +- stacklet/mcp/platform/graphql.py | 52 +++++++++++++++++- stacklet/mcp/platform/models.py | 13 +++++ stacklet/mcp/platform/tools.py | 5 +- tests/test_tools_platform.py | 79 ++++++++++++++++++++++++++- tests/testing/http.py | 13 +++++ 8 files changed, 167 insertions(+), 9 deletions(-) diff --git a/NEWS.md b/NEWS.md index 5922090..be520ea 100644 --- a/NEWS.md +++ b/NEWS.md @@ -4,6 +4,12 @@ ### Changes +- **Dataset exports download with your credentials**: Stacklet now requires sign-in to + download an export, and only the user who started it can. When the server writes files, + `platform_dataset_lookup` and `platform_dataset_export` download the finished export with + your token and return the local path in `full_results_saved_to`. A hosted server returns + the link with a `download_note` telling you to open it in your signed-in browser. + ### Fixes --- diff --git a/README.md b/README.md index 62528a8..babfc0b 100644 --- a/README.md +++ b/README.md @@ -67,7 +67,7 @@ When the MCP is run from an agent, those can be set in the `"env"` section of th The following variables are available: -- `STACKLET_MCP_DOWNLOADS_PATH`: directory for storing query result files (default: system temp directory) +- `STACKLET_MCP_DOWNLOADS_PATH`: directory for storing query result and dataset export files (default: system temp directory) - `STACKLET_MCP_ASSETDB_DATASOURCE_NAME`: the name of the AssetDB data source in Redash, which the id is looked up from on first use (default: `AssetDB`) - `STACKLET_MCP_ASSETDB_DATASOURCE`: the datasource ID for AssetDB in Redash, for a deployment that needs to name one explicitly. Unset by default, meaning look it up by name — Redash assigns the id at creation, so it differs between deployments and cannot be assumed - `STACKLET_MCP_ASSETDB_ALLOW_SAVE`: whether to enable write operations in AssetDB (default: `false`) diff --git a/stacklet/mcp/platform/dataset_info.md b/stacklet/mcp/platform/dataset_info.md index 12f2d37..4acff34 100644 --- a/stacklet/mcp/platform/dataset_info.md +++ b/stacklet/mcp/platform/dataset_info.md @@ -85,7 +85,9 @@ platform_dataset_export( **Key status fields:** `dataset_id`, `success`, `download_url`, `available_until` (24hr expiry) -**Download:** `curl -L -o file.csv "download_url"` (follows S3 redirects) +**Download:** the link needs the credentials of the user who started the export, so do not fetch it yourself. +- `full_results_saved_to` set: the server already downloaded the file to that local path. +- `download_note` set: give the user `download_url` and the note. They open it in a browser where they're signed in to Stacklet. ## Quick Examples @@ -110,6 +112,6 @@ platform_dataset_export( - Cannot use paging params (`first`, `after`) - export handles pagination - `timeout=0` returns immediately; use `platform_dataset_lookup` to monitor -- Download URLs redirect to S3 - use `curl -L` +- Only the user who started an export can download it, while signed in - Files expire after 24 hours - Use GraphQL Node IDs for node-based exports diff --git a/stacklet/mcp/platform/graphql.py b/stacklet/mcp/platform/graphql.py index ac8f5ac..bbf6f44 100644 --- a/stacklet/mcp/platform/graphql.py +++ b/stacklet/mcp/platform/graphql.py @@ -8,10 +8,13 @@ """ import asyncio +import os import re import time +from pathlib import PurePosixPath from typing import Any, Self, cast +from urllib.parse import urlparse import httpx @@ -33,6 +36,7 @@ from ..settings import SETTINGS from ..stacklet_auth import StackletCredentials from ..utils.error import AnnotatedError +from ..utils.file import download_file from .models import ( ConnectionExport, ExportRequest, @@ -43,6 +47,12 @@ ) +HOSTED_DOWNLOAD_NOTE = ( + "Open this link in a browser where you're signed in to Stacklet. " + "It works for 24 hours and only for you." +) + + class PlatformClient: """Client for Stacklet Platform GraphQL API.""" @@ -209,7 +219,7 @@ async def wait_for_export(self, dataset_id: str, timeout_s: int) -> ConnectionEx # Always try at least once. export = await self._get_export(dataset_id) if export.completed: - return export + return await self._deliver(export) # Aim for the final attempt to happen at cutoff time. remaining_s = cutoff - time.monotonic() @@ -233,6 +243,46 @@ async def _get_export(self, dataset_id: str) -> ConnectionExport: ) return ConnectionExport(**fields) + async def _deliver(self, export: ConnectionExport) -> ConnectionExport: + """Make a completed export's file reachable by the caller. + + The download URL only works with the credentials of the user who started the + export, so the server fetches it when the caller can read the result, and + otherwise tells the caller to open it in their own signed-in browser. + """ + if not export.download_url: + return export + if not SETTINGS.downloads_enabled: + return export.model_copy(update={"download_note": HOSTED_DOWNLOAD_NOTE}) + path = await self._download(export.dataset_id, export.download_url) + return export.model_copy(update={"full_results_saved_to": path}) + + async def _download(self, dataset_id: str, url: str) -> str: + """Stream an export file into the downloads directory, returning its path.""" + async with self.session.stream("GET", url) as response: + if response.status_code != 200: + await response.aread() + raise AnnotatedError( + problem=( + f"Downloading dataset export {dataset_id} failed " + f"with HTTP {response.status_code}" + ), + likely_cause="the export expired, or these credentials cannot read it", + next_steps="start a new export with platform_dataset_export", + original_error=response.text, + ) + + suffix = PurePosixPath(urlparse(url).path).suffix or ".csv" + with download_file("wb", "export_", suffix) as f: + try: + async for chunk in response.aiter_bytes(): + f.write(chunk) + except BaseException: + f.close() + os.unlink(f.name) + raise + return f.name + Q_GET_EXPORT = """ query getExport($id: ID!) { node(id: $id) { diff --git a/stacklet/mcp/platform/models.py b/stacklet/mcp/platform/models.py index 00ee1f9..1fcc20d 100644 --- a/stacklet/mcp/platform/models.py +++ b/stacklet/mcp/platform/models.py @@ -150,3 +150,16 @@ class ConnectionExport(BaseModel): available_until: datetime | None = Field( None, validation_alias="availableUntil", description="When download URL expires" ) + + # Exactly one of these is set for a successful export: the server downloads the file + # when the caller shares its filesystem, and otherwise says how to use the link. + full_results_saved_to: str | None = Field( + None, + description=( + "Local path where the server saved the exported file. Null when the server " + "does not write files, in which case see download_note." + ), + ) + download_note: str | None = Field( + None, description="How to use download_url when the server did not download it" + ) diff --git a/stacklet/mcp/platform/tools.py b/stacklet/mcp/platform/tools.py index b1bf6a0..d7025b8 100644 --- a/stacklet/mcp/platform/tools.py +++ b/stacklet/mcp/platform/tools.py @@ -251,11 +251,12 @@ async def platform_dataset_lookup( Export states: - Processing: Export is running (shows progress if available) - - Complete: Ready for download (includes download_url and expiry time) + - Complete: Ready for download (includes download_url and expiry time, plus either + full_results_saved_to or download_note) - Failed: Export encountered an error Set timeout > 0 to wait for completion, or timeout=0 for immediate status check. - Download URLs are temporary and expire after a few hours. + Download URLs expire after 24 hours and work only for the user who started the export. """ client = PlatformClient.get(ctx) return await client.wait_for_export(dataset_id, timeout) diff --git a/tests/test_tools_platform.py b/tests/test_tools_platform.py index e41cc51..24d5750 100644 --- a/tests/test_tools_platform.py +++ b/tests/test_tools_platform.py @@ -17,11 +17,12 @@ from graphql import build_schema, parse from mcp.types import ToolAnnotations -from stacklet.mcp.platform.graphql import PlatformClient, has_mutations +from stacklet.mcp.platform.graphql import HOSTED_DOWNLOAD_NOTE, PlatformClient, has_mutations from stacklet.mcp.platform.models import ExportParam from stacklet.mcp.platform.tools import tools +from stacklet.mcp.settings import SETTINGS -from .testing.http import ExpectRequest +from .testing.http import ExpectRequest, MockHTTPXResponse from .testing.mcp import MCPBearerTest, MCPTest, json_guard_parametrize @@ -348,6 +349,7 @@ def graphql_field_error(message: str, field_path: list, line: int = 1, column: i class PlatformDatasetTest(MCPBearerTest): DATASET_ID = "node-123" + CSV = "id\nabc\n" @staticmethod def dataset_result(dataset_id, started=False, succeeded=None): @@ -435,7 +437,37 @@ def assert_result(self, result, started=False, succeeded=None): else: expected["message"] = "meh." - assert result.json() == expected + actual = result.json() + saved_to = actual.pop("full_results_saved_to") + note = actual.pop("download_note") + assert actual == expected + + # A successful export reaches the caller by exactly one route: on disk when the + # server writes files, as a note about the link when it doesn't. + if succeeded and SETTINGS.downloads_enabled: + assert note is None + with open(saved_to) as f: + assert f.read() == self.CSV + elif succeeded: + assert saved_to is None + assert note == HOSTED_DOWNLOAD_NOTE + else: + assert saved_to is None + assert note is None + if not (succeeded and SETTINGS.downloads_enabled): + assert list(SETTINGS.downloads_path.iterdir()) == [] + + def expect_download(self, status_code=200, response=None): + """Create expectation for the export file download, made with the bearer token.""" + return ExpectRequest( + "https://example.com/x.csv", + status_code=status_code, + response=self.CSV if response is None else response, + ) + + def delivery(self, succeeded): + """Expected requests that deliver a completed export's file.""" + return [self.expect_download()] if succeeded and SETTINGS.downloads_enabled else [] def expect_start_export(self, columns, connection="someConnection", node_id=None, params=None): """Create expectation for the export mutation request.""" @@ -552,6 +584,7 @@ async def test_response_complete(self): with self.http.expect( self.expect_start_export(columns), self.expect_get_export(dataset), + *self.delivery(succeeded=True), ): result = await self.assert_call( { @@ -640,6 +673,7 @@ async def test_complete_timeout_immediate(self, succeeded, mangle, value, async_ with self.http.expect( self.expect_get_export(complete), + *self.delivery(succeeded), ): result = await self.assert_call( {"dataset_id": self.DATASET_ID, "timeout": mangle(value)} @@ -659,6 +693,7 @@ async def test_complete_timeout_delayed(self, succeeded, mangle, value, async_sl self.expect_get_export(incomplete), self.expect_get_export(incomplete), self.expect_get_export(complete), + *self.delivery(succeeded), ): result = await self.assert_call( {"dataset_id": self.DATASET_ID, "timeout": mangle(value)} @@ -678,6 +713,44 @@ async def test_not_found(self): assert result.text.startswith(f"Dataset export {self.DATASET_ID} not found") + @pytest.mark.parametrize("succeeded", [True, False]) + async def test_hosted_does_not_download(self, override_setting, succeeded): + override_setting("downloads_enabled", False) + complete = self.dataset_result(self.DATASET_ID, started=True, succeeded=succeeded) + + with self.http.expect(self.expect_get_export(complete)): + result = await self.assert_call({"dataset_id": self.DATASET_ID}) + + self.assert_result(result, started=True, succeeded=succeeded) + + @pytest.mark.parametrize("status_code", [401, 404]) + async def test_download_refused(self, status_code): + complete = self.dataset_result(self.DATASET_ID, started=True, succeeded=True) + body = json.dumps({"error": "Export not found or expired."}) + + with self.http.expect( + self.expect_get_export(complete), + self.expect_download(status_code=status_code, response=body), + ): + result = await self.assert_call({"dataset_id": self.DATASET_ID}, error=True) + + assert f"failed with HTTP {status_code}" in result.text + assert "Export not found or expired." in result.text + assert list(SETTINGS.downloads_path.iterdir()) == [] + + async def test_download_interrupted(self, monkeypatch): + async def broken_stream(self): + yield b"id\n" + raise httpx.ReadError("connection lost") + + monkeypatch.setattr(MockHTTPXResponse, "aiter_bytes", broken_stream) + complete = self.dataset_result(self.DATASET_ID, started=True, succeeded=True) + + with self.http.expect(self.expect_get_export(complete), self.expect_download()): + await self.assert_call({"dataset_id": self.DATASET_ID}, error=True) + + assert list(SETTINGS.downloads_path.iterdir()) == [] + class TestToolAnnotations: """The behavioural hints hosts show when asking to approve a tool call.""" diff --git a/tests/testing/http.py b/tests/testing/http.py index 1138837..a9bf18f 100644 --- a/tests/testing/http.py +++ b/tests/testing/http.py @@ -7,6 +7,7 @@ import json +from contextlib import asynccontextmanager from typing import Any import httpx @@ -31,6 +32,12 @@ def text(self): def json(self): return json.loads(self._data) + async def aread(self): + return self.content + + async def aiter_bytes(self): + yield self.content + def raise_for_status(self): if self.status_code >= 400: raise httpx.HTTPStatusError( @@ -112,7 +119,13 @@ async def mock_request(self, method, url, **kwargs): auth_check_func(self, mock_stacklet_credentials) return controller.next_request().respond(method, url, **kwargs) + @asynccontextmanager + async def mock_stream(self, method, url, **kwargs): + auth_check_func(self, mock_stacklet_credentials) + yield controller.next_request().respond(method, url, **kwargs) + monkeypatch.setattr("httpx.AsyncClient.request", mock_request) + monkeypatch.setattr("httpx.AsyncClient.stream", mock_stream) return controller From b83327d47ea5294c89325d62cf7bd5a21f7e775d Mon Sep 17 00:00:00 2001 From: John Anderson Date: Wed, 7 Oct 2026 13:22:02 -0400 Subject: [PATCH 04/16] test: expired exports and failed downloads keep what the caller needs --- tests/test_tools_platform.py | 39 ++++++++++++++++++++++++++++-------- 1 file changed, 31 insertions(+), 8 deletions(-) diff --git a/tests/test_tools_platform.py b/tests/test_tools_platform.py index 24d5750..8dbbf67 100644 --- a/tests/test_tools_platform.py +++ b/tests/test_tools_platform.py @@ -347,12 +347,16 @@ def graphql_field_error(message: str, field_path: list, line: int = 1, column: i } +AVAILABLE_UNTIL = "2099-12-07T03:15:09+00:00" +EXPIRED = "2024-12-07T03:15:09+00:00" + + class PlatformDatasetTest(MCPBearerTest): DATASET_ID = "node-123" CSV = "id\nabc\n" @staticmethod - def dataset_result(dataset_id, started=False, succeeded=None): + def dataset_result(dataset_id, started=False, succeeded=None, available_until=AVAILABLE_UNTIL): node = { "id": dataset_id, "started": None, @@ -372,7 +376,7 @@ def dataset_result(dataset_id, started=False, succeeded=None): node |= { "message": "yay!", "downloadURL": "https://example.com/x.csv", - "availableUntil": "2024-12-07T03:15:09+00:00", + "availableUntil": available_until, } else: node |= {"message": "meh."} @@ -411,7 +415,7 @@ def fancy_export_param(): }, } - def assert_result(self, result, started=False, succeeded=None): + def assert_result(self, result, started=False, succeeded=None, expired=False): """Assert that result matches expected dataset state from factory args.""" expected = { "dataset_id": self.DATASET_ID, @@ -433,7 +437,9 @@ def assert_result(self, result, started=False, succeeded=None): if succeeded: expected["message"] = "yay!" expected["download_url"] = "https://example.com/x.csv" - expected["available_until"] = "2024-12-07T03:15:09Z" + expected["available_until"] = (EXPIRED if expired else AVAILABLE_UNTIL).replace( + "+00:00", "Z" + ) else: expected["message"] = "meh." @@ -444,17 +450,18 @@ def assert_result(self, result, started=False, succeeded=None): # A successful export reaches the caller by exactly one route: on disk when the # server writes files, as a note about the link when it doesn't. - if succeeded and SETTINGS.downloads_enabled: + delivered = succeeded and not expired + if delivered and SETTINGS.downloads_enabled: assert note is None with open(saved_to) as f: assert f.read() == self.CSV - elif succeeded: + elif delivered: assert saved_to is None assert note == HOSTED_DOWNLOAD_NOTE else: assert saved_to is None assert note is None - if not (succeeded and SETTINGS.downloads_enabled): + if not (delivered and SETTINGS.downloads_enabled): assert list(SETTINGS.downloads_path.iterdir()) == [] def expect_download(self, status_code=200, response=None): @@ -736,6 +743,7 @@ async def test_download_refused(self, status_code): assert f"failed with HTTP {status_code}" in result.text assert "Export not found or expired." in result.text + assert "https://example.com/x.csv" in result.text assert list(SETTINGS.downloads_path.iterdir()) == [] async def test_download_interrupted(self, monkeypatch): @@ -747,10 +755,25 @@ async def broken_stream(self): complete = self.dataset_result(self.DATASET_ID, started=True, succeeded=True) with self.http.expect(self.expect_get_export(complete), self.expect_download()): - await self.assert_call({"dataset_id": self.DATASET_ID}, error=True) + result = await self.assert_call({"dataset_id": self.DATASET_ID}, error=True) + # The caller keeps what it needs to retry: the export ID and the link. + assert f"Downloading dataset export {self.DATASET_ID} failed" in result.text + assert "https://example.com/x.csv" in result.text assert list(SETTINGS.downloads_path.iterdir()) == [] + @pytest.mark.parametrize("downloads_enabled", [True, False]) + async def test_expired_not_delivered(self, override_setting, downloads_enabled): + override_setting("downloads_enabled", downloads_enabled) + expired = self.dataset_result( + self.DATASET_ID, started=True, succeeded=True, available_until=EXPIRED + ) + + with self.http.expect(self.expect_get_export(expired)): + result = await self.assert_call({"dataset_id": self.DATASET_ID}) + + self.assert_result(result, started=True, succeeded=True, expired=True) + class TestToolAnnotations: """The behavioural hints hosts show when asking to approve a tool call.""" From dc9cbb0086057a765c57243c4d35213bb84197a4 Mon Sep 17 00:00:00 2001 From: John Anderson Date: Wed, 7 Oct 2026 13:22:02 -0400 Subject: [PATCH 05/16] fix: skip expired exports and keep retry details when a download fails --- stacklet/mcp/platform/graphql.py | 72 +++++++++++++++++++++----------- stacklet/mcp/platform/tools.py | 9 +++- 2 files changed, 55 insertions(+), 26 deletions(-) diff --git a/stacklet/mcp/platform/graphql.py b/stacklet/mcp/platform/graphql.py index bbf6f44..2da1081 100644 --- a/stacklet/mcp/platform/graphql.py +++ b/stacklet/mcp/platform/graphql.py @@ -12,6 +12,8 @@ import re import time +from contextlib import suppress +from datetime import UTC, datetime from pathlib import PurePosixPath from typing import Any, Self, cast from urllib.parse import urlparse @@ -250,7 +252,10 @@ async def _deliver(self, export: ConnectionExport) -> ConnectionExport: export, so the server fetches it when the caller can read the result, and otherwise tells the caller to open it in their own signed-in browser. """ - if not export.download_url: + # The platform keeps returning the URL after the file has expired. + if not export.download_url or ( + export.available_until and export.available_until <= datetime.now(UTC) + ): return export if not SETTINGS.downloads_enabled: return export.model_copy(update={"download_note": HOSTED_DOWNLOAD_NOTE}) @@ -259,29 +264,48 @@ async def _deliver(self, export: ConnectionExport) -> ConnectionExport: async def _download(self, dataset_id: str, url: str) -> str: """Stream an export file into the downloads directory, returning its path.""" - async with self.session.stream("GET", url) as response: - if response.status_code != 200: - await response.aread() - raise AnnotatedError( - problem=( - f"Downloading dataset export {dataset_id} failed " - f"with HTTP {response.status_code}" - ), - likely_cause="the export expired, or these credentials cannot read it", - next_steps="start a new export with platform_dataset_export", - original_error=response.text, - ) - - suffix = PurePosixPath(urlparse(url).path).suffix or ".csv" - with download_file("wb", "export_", suffix) as f: - try: - async for chunk in response.aiter_bytes(): - f.write(chunk) - except BaseException: - f.close() - os.unlink(f.name) - raise - return f.name + next_steps = ( + f"retry with platform_dataset_lookup, or give the user {url} " + "to open in a browser where they're signed in to Stacklet" + ) + try: + async with self.session.stream("GET", url) as response: + if response.status_code != 200: + await response.aread() + raise AnnotatedError( + problem=( + f"Downloading dataset export {dataset_id} failed " + f"with HTTP {response.status_code}" + ), + likely_cause="the export expired, or these credentials cannot read it", + next_steps=next_steps, + original_error=response.text, + ) + return await self._save(response, PurePosixPath(urlparse(url).path).suffix) + except httpx.HTTPError as e: + raise AnnotatedError( + problem=f"Downloading dataset export {dataset_id} failed", + likely_cause="a network error while fetching the file", + next_steps=next_steps, + original_error=str(e), + ) from e + + async def _save(self, response: httpx.Response, suffix: str) -> str: + """Write a response body to a new downloads file, removing it if anything fails. + + File I/O runs in a thread so a large export does not block the event loop. + """ + f = await asyncio.to_thread(download_file, "wb", "export_", suffix or ".csv") + try: + async for chunk in response.aiter_bytes(): + await asyncio.to_thread(f.write, chunk) + await asyncio.to_thread(f.close) + except BaseException: + with suppress(OSError): + f.close() + os.unlink(f.name) + raise + return f.name Q_GET_EXPORT = """ query getExport($id: ID!) { diff --git a/stacklet/mcp/platform/tools.py b/stacklet/mcp/platform/tools.py index d7025b8..9f641aa 100644 --- a/stacklet/mcp/platform/tools.py +++ b/stacklet/mcp/platform/tools.py @@ -211,7 +211,10 @@ async def platform_dataset_export( 1. Define columns mapping GraphQL fields to CSV columns 2. Optionally add filters via params 3. Export runs asynchronously - use timeout=0 to return immediately - 4. Use platform_dataset_lookup() to check progress and get download URL + 4. Use platform_dataset_lookup() to check progress and get the result + + A completed export's file is downloaded before the call returns when the server writes + files, so a large export takes longer than the timeout alone suggests. """ dataset_input = ExportRequest( connection_field=connection_field, @@ -255,7 +258,9 @@ async def platform_dataset_lookup( full_results_saved_to or download_note) - Failed: Export encountered an error - Set timeout > 0 to wait for completion, or timeout=0 for immediate status check. + Set timeout > 0 to wait for completion, or timeout=0 for an immediate status check. + Every lookup of a completed export downloads its file again when the server writes + files, so the call takes as long as that download. Download URLs expire after 24 hours and work only for the user who started the export. """ client = PlatformClient.get(ctx) From d5a778f4f5c884eea14e1083baebcf1d9e67729e Mon Sep 17 00:00:00 2001 From: John Anderson Date: Wed, 7 Oct 2026 13:24:05 -0400 Subject: [PATCH 06/16] refactor: tighten export download docs and stream helper --- stacklet/mcp/platform/graphql.py | 41 +++++++++++++------------------- stacklet/mcp/platform/models.py | 5 ++-- tests/test_tools_platform.py | 9 ++----- 3 files changed, 21 insertions(+), 34 deletions(-) diff --git a/stacklet/mcp/platform/graphql.py b/stacklet/mcp/platform/graphql.py index 2da1081..d262199 100644 --- a/stacklet/mcp/platform/graphql.py +++ b/stacklet/mcp/platform/graphql.py @@ -246,12 +246,7 @@ async def _get_export(self, dataset_id: str) -> ConnectionExport: return ConnectionExport(**fields) async def _deliver(self, export: ConnectionExport) -> ConnectionExport: - """Make a completed export's file reachable by the caller. - - The download URL only works with the credentials of the user who started the - export, so the server fetches it when the caller can read the result, and - otherwise tells the caller to open it in their own signed-in browser. - """ + """Download a completed export's file, or say how to use its link instead.""" # The platform keeps returning the URL after the file has expired. if not export.download_url or ( export.available_until and export.available_until <= datetime.now(UTC) @@ -281,7 +276,7 @@ async def _download(self, dataset_id: str, url: str) -> str: next_steps=next_steps, original_error=response.text, ) - return await self._save(response, PurePosixPath(urlparse(url).path).suffix) + return await _save_stream(response, PurePosixPath(urlparse(url).path).suffix) except httpx.HTTPError as e: raise AnnotatedError( problem=f"Downloading dataset export {dataset_id} failed", @@ -290,23 +285,6 @@ async def _download(self, dataset_id: str, url: str) -> str: original_error=str(e), ) from e - async def _save(self, response: httpx.Response, suffix: str) -> str: - """Write a response body to a new downloads file, removing it if anything fails. - - File I/O runs in a thread so a large export does not block the event loop. - """ - f = await asyncio.to_thread(download_file, "wb", "export_", suffix or ".csv") - try: - async for chunk in response.aiter_bytes(): - await asyncio.to_thread(f.write, chunk) - await asyncio.to_thread(f.close) - except BaseException: - with suppress(OSError): - f.close() - os.unlink(f.name) - raise - return f.name - Q_GET_EXPORT = """ query getExport($id: ID!) { node(id: $id) { @@ -347,6 +325,21 @@ async def _query(self, query: str, variables: dict[str, Any]) -> GraphQLQueryRes raise Exception(f"Unexpected response: {response.text}") +async def _save_stream(response: httpx.Response, suffix: str) -> str: + """Write a response body to a new downloads file, removing it if anything fails.""" + f = await asyncio.to_thread(download_file, "wb", "export_", suffix or ".csv") + try: + async for chunk in response.aiter_bytes(): + await asyncio.to_thread(f.write, chunk) + await asyncio.to_thread(f.close) + except BaseException: + with suppress(OSError): + f.close() + os.unlink(f.name) + raise + return f.name + + def has_mutations(doc: DocumentNode) -> bool: """Return whether a GraphQL document calls mutations.""" operations = {dd.operation for dd in doc.definitions} diff --git a/stacklet/mcp/platform/models.py b/stacklet/mcp/platform/models.py index 1fcc20d..03605d1 100644 --- a/stacklet/mcp/platform/models.py +++ b/stacklet/mcp/platform/models.py @@ -151,13 +151,12 @@ class ConnectionExport(BaseModel): None, validation_alias="availableUntil", description="When download URL expires" ) - # Exactly one of these is set for a successful export: the server downloads the file - # when the caller shares its filesystem, and otherwise says how to use the link. + # A succeeded, unexpired export sets exactly one of these. full_results_saved_to: str | None = Field( None, description=( "Local path where the server saved the exported file. Null when the server " - "does not write files, in which case see download_note." + "does not write files (see download_note) or the export has expired." ), ) download_note: str | None = Field( diff --git a/tests/test_tools_platform.py b/tests/test_tools_platform.py index 8dbbf67..16468d9 100644 --- a/tests/test_tools_platform.py +++ b/tests/test_tools_platform.py @@ -448,20 +448,15 @@ def assert_result(self, result, started=False, succeeded=None, expired=False): note = actual.pop("download_note") assert actual == expected - # A successful export reaches the caller by exactly one route: on disk when the - # server writes files, as a note about the link when it doesn't. + # A delivered export reaches the caller by exactly one route. delivered = succeeded and not expired if delivered and SETTINGS.downloads_enabled: assert note is None with open(saved_to) as f: assert f.read() == self.CSV - elif delivered: - assert saved_to is None - assert note == HOSTED_DOWNLOAD_NOTE else: assert saved_to is None - assert note is None - if not (delivered and SETTINGS.downloads_enabled): + assert note == (HOSTED_DOWNLOAD_NOTE if delivered else None) assert list(SETTINGS.downloads_path.iterdir()) == [] def expect_download(self, status_code=200, response=None): From 4309cd6174c8f323574531cda473454faff56b07 Mon Sep 17 00:00:00 2001 From: John Anderson Date: Wed, 7 Oct 2026 13:26:07 -0400 Subject: [PATCH 07/16] test: an expired export says so instead of returning a dead link --- tests/test_tools_platform.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/tests/test_tools_platform.py b/tests/test_tools_platform.py index 16468d9..74cbfa2 100644 --- a/tests/test_tools_platform.py +++ b/tests/test_tools_platform.py @@ -17,7 +17,12 @@ from graphql import build_schema, parse from mcp.types import ToolAnnotations -from stacklet.mcp.platform.graphql import HOSTED_DOWNLOAD_NOTE, PlatformClient, has_mutations +from stacklet.mcp.platform.graphql import ( + EXPIRED_DOWNLOAD_NOTE, + HOSTED_DOWNLOAD_NOTE, + PlatformClient, + has_mutations, +) from stacklet.mcp.platform.models import ExportParam from stacklet.mcp.platform.tools import tools from stacklet.mcp.settings import SETTINGS @@ -456,7 +461,10 @@ def assert_result(self, result, started=False, succeeded=None, expired=False): assert f.read() == self.CSV else: assert saved_to is None - assert note == (HOSTED_DOWNLOAD_NOTE if delivered else None) + if delivered: + assert note == HOSTED_DOWNLOAD_NOTE + else: + assert note == (EXPIRED_DOWNLOAD_NOTE if expired else None) assert list(SETTINGS.downloads_path.iterdir()) == [] def expect_download(self, status_code=200, response=None): From b80bc2e2e8f5729b75f00612dbc35e73f6176dcf Mon Sep 17 00:00:00 2001 From: John Anderson Date: Wed, 7 Oct 2026 13:26:07 -0400 Subject: [PATCH 08/16] fix: tell the caller when an export has expired --- stacklet/mcp/platform/graphql.py | 12 +++++++----- stacklet/mcp/platform/models.py | 4 ++-- stacklet/mcp/platform/tools.py | 2 +- 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/stacklet/mcp/platform/graphql.py b/stacklet/mcp/platform/graphql.py index d262199..50dd7fe 100644 --- a/stacklet/mcp/platform/graphql.py +++ b/stacklet/mcp/platform/graphql.py @@ -54,6 +54,8 @@ "It works for 24 hours and only for you." ) +EXPIRED_DOWNLOAD_NOTE = "This export has expired. Start a new one with platform_dataset_export." + class PlatformClient: """Client for Stacklet Platform GraphQL API.""" @@ -247,11 +249,11 @@ async def _get_export(self, dataset_id: str) -> ConnectionExport: async def _deliver(self, export: ConnectionExport) -> ConnectionExport: """Download a completed export's file, or say how to use its link instead.""" - # The platform keeps returning the URL after the file has expired. - if not export.download_url or ( - export.available_until and export.available_until <= datetime.now(UTC) - ): + if not export.download_url: return export + # The platform keeps returning the URL after the file has expired. + if export.available_until and export.available_until <= datetime.now(UTC): + return export.model_copy(update={"download_note": EXPIRED_DOWNLOAD_NOTE}) if not SETTINGS.downloads_enabled: return export.model_copy(update={"download_note": HOSTED_DOWNLOAD_NOTE}) path = await self._download(export.dataset_id, export.download_url) @@ -327,7 +329,7 @@ async def _query(self, query: str, variables: dict[str, Any]) -> GraphQLQueryRes async def _save_stream(response: httpx.Response, suffix: str) -> str: """Write a response body to a new downloads file, removing it if anything fails.""" - f = await asyncio.to_thread(download_file, "wb", "export_", suffix or ".csv") + f = download_file("wb", "export_", suffix or ".csv") try: async for chunk in response.aiter_bytes(): await asyncio.to_thread(f.write, chunk) diff --git a/stacklet/mcp/platform/models.py b/stacklet/mcp/platform/models.py index 03605d1..deea648 100644 --- a/stacklet/mcp/platform/models.py +++ b/stacklet/mcp/platform/models.py @@ -151,12 +151,12 @@ class ConnectionExport(BaseModel): None, validation_alias="availableUntil", description="When download URL expires" ) - # A succeeded, unexpired export sets exactly one of these. + # A succeeded export sets exactly one of these. full_results_saved_to: str | None = Field( None, description=( "Local path where the server saved the exported file. Null when the server " - "does not write files (see download_note) or the export has expired." + "does not write files or the export has expired, in which case see download_note." ), ) download_note: str | None = Field( diff --git a/stacklet/mcp/platform/tools.py b/stacklet/mcp/platform/tools.py index 9f641aa..4281e67 100644 --- a/stacklet/mcp/platform/tools.py +++ b/stacklet/mcp/platform/tools.py @@ -255,7 +255,7 @@ async def platform_dataset_lookup( Export states: - Processing: Export is running (shows progress if available) - Complete: Ready for download (includes download_url and expiry time, plus either - full_results_saved_to or download_note) + full_results_saved_to or download_note; an expired export has only the note) - Failed: Export encountered an error Set timeout > 0 to wait for completion, or timeout=0 for an immediate status check. From 538e2651c82480ce19376a82ed872293ccd931dc Mon Sep 17 00:00:00 2001 From: John Anderson Date: Wed, 7 Oct 2026 13:27:14 -0400 Subject: [PATCH 09/16] docs: describe the expired-export note --- stacklet/mcp/platform/dataset_info.md | 3 ++- stacklet/mcp/platform/models.py | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/stacklet/mcp/platform/dataset_info.md b/stacklet/mcp/platform/dataset_info.md index 4acff34..7f0539e 100644 --- a/stacklet/mcp/platform/dataset_info.md +++ b/stacklet/mcp/platform/dataset_info.md @@ -87,7 +87,8 @@ platform_dataset_export( **Download:** the link needs the credentials of the user who started the export, so do not fetch it yourself. - `full_results_saved_to` set: the server already downloaded the file to that local path. -- `download_note` set: give the user `download_url` and the note. They open it in a browser where they're signed in to Stacklet. +- `download_note` set and `available_until` still ahead: give the user `download_url` and the note. They open it in a browser where they're signed in to Stacklet. +- `available_until` passed: the link is dead. Start a new export instead of sharing it. ## Quick Examples diff --git a/stacklet/mcp/platform/models.py b/stacklet/mcp/platform/models.py index deea648..3a87b14 100644 --- a/stacklet/mcp/platform/models.py +++ b/stacklet/mcp/platform/models.py @@ -160,5 +160,5 @@ class ConnectionExport(BaseModel): ), ) download_note: str | None = Field( - None, description="How to use download_url when the server did not download it" + None, description="Why the server did not download the file, and what to do instead" ) From cd6afee33ec1747381ba38efc6051f30d57ca613 Mon Sep 17 00:00:00 2001 From: John Anderson Date: Wed, 7 Oct 2026 14:47:42 -0400 Subject: [PATCH 10/16] docs: describe dataset exports alongside query results for the downloads setting --- CLAUDE.md | 4 ++++ stacklet/mcp/platform/models.py | 2 +- stacklet/mcp/settings.py | 8 ++++---- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 70fb9b1..e0d9cb1 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -153,6 +153,10 @@ available by exactly one of two routes, depending on `downloads_enabled`: attached to the tool response as a resource. Hosted deployments must disable downloads — a caller elsewhere cannot read the server's filesystem, so the files would only accumulate. +Dataset export tools follow the same switch. Enabled, a completed export is downloaded with +the caller's token to `export_*` in the downloads directory (`full_results_saved_to`). +Disabled, the tool returns the link with a `download_note` to open it in a signed-in browser. + **External Dependencies:** - Documentation files are fetched from the live Stacklet docs service at runtime - Redash endpoint is derived by replacing "api." with "redash." in the platform endpoint diff --git a/stacklet/mcp/platform/models.py b/stacklet/mcp/platform/models.py index 3a87b14..bb5cf92 100644 --- a/stacklet/mcp/platform/models.py +++ b/stacklet/mcp/platform/models.py @@ -151,7 +151,7 @@ class ConnectionExport(BaseModel): None, validation_alias="availableUntil", description="When download URL expires" ) - # A succeeded export sets exactly one of these. + # A succeeded export with a download URL sets exactly one of these. full_results_saved_to: str | None = Field( None, description=( diff --git a/stacklet/mcp/settings.py b/stacklet/mcp/settings.py index 0d7d162..93d2764 100644 --- a/stacklet/mcp/settings.py +++ b/stacklet/mcp/settings.py @@ -23,10 +23,10 @@ class Settings(BaseSettings): downloads_enabled: bool = Field( default=True, description=( - "Write complete query results to downloads_path. Disable for hosted " - "deployments, where the caller cannot read the server's filesystem and " - "the files would accumulate unbounded; full results are then attached " - "to the tool response instead." + "Write complete query results and dataset exports to downloads_path. " + "Disable for hosted deployments, where the caller cannot read the server's " + "filesystem and the files would accumulate unbounded; query results are then " + "attached to the tool response, and exports return their link with a note." ), ) From c2d0861fe4122721cb3ef8f3224a889749898388 Mon Sep 17 00:00:00 2001 From: John Anderson Date: Wed, 7 Oct 2026 14:48:25 -0400 Subject: [PATCH 11/16] docs: cover dataset exports in the downloads settings --- CLAUDE.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index e0d9cb1..fc3297d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -134,8 +134,8 @@ The server requires Stacklet credentials configured through one of: **Server Settings:** Additional configuration via environment variables with `STACKLET_MCP_` prefix: -- `STACKLET_MCP_DOWNLOADS_PATH` (default: system temp directory) - Directory for storing query result files -- `STACKLET_MCP_DOWNLOADS_ENABLED` (default: true) - Write query result files; set false for hosted deployments +- `STACKLET_MCP_DOWNLOADS_PATH` (default: system temp directory) - Directory for storing query result and dataset export files +- `STACKLET_MCP_DOWNLOADS_ENABLED` (default: true) - Write query result and dataset export files; set false for hosted deployments - `STACKLET_MCP_ASSETDB_DATASOURCE_NAME` (default: AssetDB) - Name the data source id is looked up from on first use - `STACKLET_MCP_ASSETDB_DATASOURCE` (default: unset) - AssetDB data source ID, when a deployment needs to name one explicitly; unset means look it up by name - `STACKLET_MCP_ASSETDB_ALLOW_SAVE` (default: false) - Enable query save/update functionality @@ -156,6 +156,7 @@ available by exactly one of two routes, depending on `downloads_enabled`: Dataset export tools follow the same switch. Enabled, a completed export is downloaded with the caller's token to `export_*` in the downloads directory (`full_results_saved_to`). Disabled, the tool returns the link with a `download_note` to open it in a signed-in browser. +An expired export returns only a `download_note` either way. **External Dependencies:** - Documentation files are fetched from the live Stacklet docs service at runtime From 50ba12d51b61382ff2e912f0f5f06f83a2d4678b Mon Sep 17 00:00:00 2001 From: John Anderson Date: Wed, 7 Oct 2026 15:03:20 -0400 Subject: [PATCH 12/16] refactor: decide export delivery in the tools layer and share httpx error handling --- stacklet/mcp/platform/graphql.py | 71 +++++++++----------------------- stacklet/mcp/platform/tools.py | 25 ++++++++++- tests/test_tools_platform.py | 60 ++++++++++++++------------- 3 files changed, 75 insertions(+), 81 deletions(-) diff --git a/stacklet/mcp/platform/graphql.py b/stacklet/mcp/platform/graphql.py index 50dd7fe..80590d2 100644 --- a/stacklet/mcp/platform/graphql.py +++ b/stacklet/mcp/platform/graphql.py @@ -12,11 +12,7 @@ import re import time -from contextlib import suppress -from datetime import UTC, datetime -from pathlib import PurePosixPath from typing import Any, Self, cast -from urllib.parse import urlparse import httpx @@ -49,14 +45,6 @@ ) -HOSTED_DOWNLOAD_NOTE = ( - "Open this link in a browser where you're signed in to Stacklet. " - "It works for 24 hours and only for you." -) - -EXPIRED_DOWNLOAD_NOTE = "This export has expired. Start a new one with platform_dataset_export." - - class PlatformClient: """Client for Stacklet Platform GraphQL API.""" @@ -223,7 +211,7 @@ async def wait_for_export(self, dataset_id: str, timeout_s: int) -> ConnectionEx # Always try at least once. export = await self._get_export(dataset_id) if export.completed: - return await self._deliver(export) + return export # Aim for the final attempt to happen at cutoff time. remaining_s = cutoff - time.monotonic() @@ -247,44 +235,27 @@ async def _get_export(self, dataset_id: str) -> ConnectionExport: ) return ConnectionExport(**fields) - async def _deliver(self, export: ConnectionExport) -> ConnectionExport: - """Download a completed export's file, or say how to use its link instead.""" - if not export.download_url: - return export - # The platform keeps returning the URL after the file has expired. - if export.available_until and export.available_until <= datetime.now(UTC): - return export.model_copy(update={"download_note": EXPIRED_DOWNLOAD_NOTE}) - if not SETTINGS.downloads_enabled: - return export.model_copy(update={"download_note": HOSTED_DOWNLOAD_NOTE}) - path = await self._download(export.dataset_id, export.download_url) - return export.model_copy(update={"full_results_saved_to": path}) - - async def _download(self, dataset_id: str, url: str) -> str: + async def download_export(self, dataset_id: str, url: str) -> str: """Stream an export file into the downloads directory, returning its path.""" - next_steps = ( - f"retry with platform_dataset_lookup, or give the user {url} " - "to open in a browser where they're signed in to Stacklet" - ) try: async with self.session.stream("GET", url) as response: if response.status_code != 200: await response.aread() - raise AnnotatedError( - problem=( - f"Downloading dataset export {dataset_id} failed " - f"with HTTP {response.status_code}" - ), - likely_cause="the export expired, or these credentials cannot read it", - next_steps=next_steps, - original_error=response.text, - ) - return await _save_stream(response, PurePosixPath(urlparse(url).path).suffix) + response.raise_for_status() + return await _save_stream(response) except httpx.HTTPError as e: + detail = e.response.text if isinstance(e, httpx.HTTPStatusError) else str(e) raise AnnotatedError( problem=f"Downloading dataset export {dataset_id} failed", - likely_cause="a network error while fetching the file", - next_steps=next_steps, - original_error=str(e), + likely_cause=( + "the export expired, these credentials cannot read it, " + "or the connection dropped" + ), + next_steps=( + f"retry with platform_dataset_lookup, or give the user {url} " + "to open in a browser where they're signed in to Stacklet" + ), + original_error=detail, ) from e Q_GET_EXPORT = """ @@ -327,16 +298,14 @@ async def _query(self, query: str, variables: dict[str, Any]) -> GraphQLQueryRes raise Exception(f"Unexpected response: {response.text}") -async def _save_stream(response: httpx.Response, suffix: str) -> str: - """Write a response body to a new downloads file, removing it if anything fails.""" - f = download_file("wb", "export_", suffix or ".csv") +async def _save_stream(response: httpx.Response) -> str: + """Write a response body to a new downloads CSV, removing it if anything fails.""" + f = download_file("wb", "export_", ".csv") try: - async for chunk in response.aiter_bytes(): - await asyncio.to_thread(f.write, chunk) - await asyncio.to_thread(f.close) + with f: + async for chunk in response.aiter_bytes(): + f.write(chunk) except BaseException: - with suppress(OSError): - f.close() os.unlink(f.name) raise return f.name diff --git a/stacklet/mcp/platform/tools.py b/stacklet/mcp/platform/tools.py index 4281e67..749badc 100644 --- a/stacklet/mcp/platform/tools.py +++ b/stacklet/mcp/platform/tools.py @@ -3,6 +3,7 @@ # Copyright (c) 2025-2026 Stacklet, Inc. # +from datetime import UTC, datetime from typing import Annotated, Any from fastmcp import Context @@ -25,6 +26,26 @@ ) +HOSTED_DOWNLOAD_NOTE = ( + "Open this link in a browser where you're signed in to Stacklet. " + "It works only for you, until available_until." +) +EXPIRED_DOWNLOAD_NOTE = "This export has expired. Start a new one with platform_dataset_export." + + +async def _deliver(client: PlatformClient, export: ConnectionExport) -> ConnectionExport: + """Download a completed export's file, or say how to use its link instead.""" + if not export.completed or not export.download_url: + return export + # The platform keeps returning the URL after the file has expired. + if export.available_until and export.available_until <= datetime.now(UTC): + return export.model_copy(update={"download_note": EXPIRED_DOWNLOAD_NOTE}) + if not SETTINGS.downloads_enabled: + return export.model_copy(update={"download_note": HOSTED_DOWNLOAD_NOTE}) + path = await client.download_export(export.dataset_id, export.download_url) + return export.model_copy(update={"full_results_saved_to": path}) + + def tools() -> list[Tool]: """List of available Platform tools.""" return [ @@ -225,7 +246,7 @@ async def platform_dataset_export( client = PlatformClient.get(ctx) dataset_id = await client.start_export(dataset_input) - return await client.wait_for_export(dataset_id, timeout) + return await _deliver(client, await client.wait_for_export(dataset_id, timeout)) @json_guard @@ -264,4 +285,4 @@ async def platform_dataset_lookup( Download URLs expire after 24 hours and work only for the user who started the export. """ client = PlatformClient.get(ctx) - return await client.wait_for_export(dataset_id, timeout) + return await _deliver(client, await client.wait_for_export(dataset_id, timeout)) diff --git a/tests/test_tools_platform.py b/tests/test_tools_platform.py index 74cbfa2..6ea021b 100644 --- a/tests/test_tools_platform.py +++ b/tests/test_tools_platform.py @@ -17,14 +17,9 @@ from graphql import build_schema, parse from mcp.types import ToolAnnotations -from stacklet.mcp.platform.graphql import ( - EXPIRED_DOWNLOAD_NOTE, - HOSTED_DOWNLOAD_NOTE, - PlatformClient, - has_mutations, -) +from stacklet.mcp.platform.graphql import PlatformClient, has_mutations from stacklet.mcp.platform.models import ExportParam -from stacklet.mcp.platform.tools import tools +from stacklet.mcp.platform.tools import EXPIRED_DOWNLOAD_NOTE, HOSTED_DOWNLOAD_NOTE, tools from stacklet.mcp.settings import SETTINGS from .testing.http import ExpectRequest, MockHTTPXResponse @@ -420,7 +415,9 @@ def fancy_export_param(): }, } - def assert_result(self, result, started=False, succeeded=None, expired=False): + def assert_result( + self, result, started=False, succeeded=None, expired=False, downloads_enabled=True + ): """Assert that result matches expected dataset state from factory args.""" expected = { "dataset_id": self.DATASET_ID, @@ -455,7 +452,7 @@ def assert_result(self, result, started=False, succeeded=None, expired=False): # A delivered export reaches the caller by exactly one route. delivered = succeeded and not expired - if delivered and SETTINGS.downloads_enabled: + if delivered and downloads_enabled: assert note is None with open(saved_to) as f: assert f.read() == self.CSV @@ -476,8 +473,8 @@ def expect_download(self, status_code=200, response=None): ) def delivery(self, succeeded): - """Expected requests that deliver a completed export's file.""" - return [self.expect_download()] if succeeded and SETTINGS.downloads_enabled else [] + """Expected requests that deliver a completed export's file when the server writes files.""" + return [self.expect_download()] if succeeded else [] def expect_start_export(self, columns, connection="someConnection", node_id=None, params=None): """Create expectation for the export mutation request.""" @@ -723,16 +720,6 @@ async def test_not_found(self): assert result.text.startswith(f"Dataset export {self.DATASET_ID} not found") - @pytest.mark.parametrize("succeeded", [True, False]) - async def test_hosted_does_not_download(self, override_setting, succeeded): - override_setting("downloads_enabled", False) - complete = self.dataset_result(self.DATASET_ID, started=True, succeeded=succeeded) - - with self.http.expect(self.expect_get_export(complete)): - result = await self.assert_call({"dataset_id": self.DATASET_ID}) - - self.assert_result(result, started=True, succeeded=succeeded) - @pytest.mark.parametrize("status_code", [401, 404]) async def test_download_refused(self, status_code): complete = self.dataset_result(self.DATASET_ID, started=True, succeeded=True) @@ -744,7 +731,7 @@ async def test_download_refused(self, status_code): ): result = await self.assert_call({"dataset_id": self.DATASET_ID}, error=True) - assert f"failed with HTTP {status_code}" in result.text + assert f"Downloading dataset export {self.DATASET_ID} failed" in result.text assert "Export not found or expired." in result.text assert "https://example.com/x.csv" in result.text assert list(SETTINGS.downloads_path.iterdir()) == [] @@ -765,17 +752,34 @@ async def broken_stream(self): assert "https://example.com/x.csv" in result.text assert list(SETTINGS.downloads_path.iterdir()) == [] - @pytest.mark.parametrize("downloads_enabled", [True, False]) - async def test_expired_not_delivered(self, override_setting, downloads_enabled): + @pytest.mark.parametrize( + "downloads_enabled, succeeded, expired", + [ + pytest.param(False, True, False, id="hosted"), + pytest.param(False, False, False, id="hosted-failed"), + pytest.param(False, True, True, id="hosted-expired"), + pytest.param(True, True, True, id="local-expired"), + ], + ) + async def test_no_download(self, override_setting, downloads_enabled, succeeded, expired): override_setting("downloads_enabled", downloads_enabled) - expired = self.dataset_result( - self.DATASET_ID, started=True, succeeded=True, available_until=EXPIRED + complete = self.dataset_result( + self.DATASET_ID, + started=True, + succeeded=succeeded, + available_until=EXPIRED if expired else AVAILABLE_UNTIL, ) - with self.http.expect(self.expect_get_export(expired)): + with self.http.expect(self.expect_get_export(complete)): result = await self.assert_call({"dataset_id": self.DATASET_ID}) - self.assert_result(result, started=True, succeeded=True, expired=True) + self.assert_result( + result, + started=True, + succeeded=succeeded, + expired=expired, + downloads_enabled=downloads_enabled, + ) class TestToolAnnotations: From dbd9bb19328004ccf418963098a7be806bfb6bd4 Mon Sep 17 00:00:00 2001 From: John Anderson Date: Wed, 7 Oct 2026 15:08:38 -0400 Subject: [PATCH 13/16] test: a refused export download hides its HTTP status --- tests/test_tools_platform.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/test_tools_platform.py b/tests/test_tools_platform.py index 6ea021b..13b1c0c 100644 --- a/tests/test_tools_platform.py +++ b/tests/test_tools_platform.py @@ -473,7 +473,7 @@ def expect_download(self, status_code=200, response=None): ) def delivery(self, succeeded): - """Expected requests that deliver a completed export's file when the server writes files.""" + """Expected requests that download a completed export's file.""" return [self.expect_download()] if succeeded else [] def expect_start_export(self, columns, connection="someConnection", node_id=None, params=None): @@ -732,6 +732,7 @@ async def test_download_refused(self, status_code): result = await self.assert_call({"dataset_id": self.DATASET_ID}, error=True) assert f"Downloading dataset export {self.DATASET_ID} failed" in result.text + assert str(status_code) in result.text assert "Export not found or expired." in result.text assert "https://example.com/x.csv" in result.text assert list(SETTINGS.downloads_path.iterdir()) == [] From ae0ad10260132fcefa259491183aa48ff3a5d71b Mon Sep 17 00:00:00 2001 From: John Anderson Date: Wed, 7 Oct 2026 15:08:52 -0400 Subject: [PATCH 14/16] fix: keep the HTTP status in export download errors --- stacklet/mcp/platform/graphql.py | 4 ++-- tests/testing/http.py | 4 ++++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/stacklet/mcp/platform/graphql.py b/stacklet/mcp/platform/graphql.py index 80590d2..09b3a2e 100644 --- a/stacklet/mcp/platform/graphql.py +++ b/stacklet/mcp/platform/graphql.py @@ -239,12 +239,12 @@ async def download_export(self, dataset_id: str, url: str) -> str: """Stream an export file into the downloads directory, returning its path.""" try: async with self.session.stream("GET", url) as response: - if response.status_code != 200: + if not response.is_success: await response.aread() response.raise_for_status() return await _save_stream(response) except httpx.HTTPError as e: - detail = e.response.text if isinstance(e, httpx.HTTPStatusError) else str(e) + detail = f"{e}\n{e.response.text}" if isinstance(e, httpx.HTTPStatusError) else str(e) raise AnnotatedError( problem=f"Downloading dataset export {dataset_id} failed", likely_cause=( diff --git a/tests/testing/http.py b/tests/testing/http.py index a9bf18f..a7c9833 100644 --- a/tests/testing/http.py +++ b/tests/testing/http.py @@ -29,6 +29,10 @@ def content(self): def text(self): return self._data + @property + def is_success(self): + return 200 <= self.status_code < 300 + def json(self): return json.loads(self._data) From ddaf2a5ab2efac61ee8ad6238a9b69d4445c41fd Mon Sep 17 00:00:00 2001 From: John Anderson Date: Wed, 7 Oct 2026 15:28:14 -0400 Subject: [PATCH 15/16] test: platform_dataset_lookup writes files but is annotated read-only --- tests/test_tools_platform.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/test_tools_platform.py b/tests/test_tools_platform.py index 13b1c0c..f33a87d 100644 --- a/tests/test_tools_platform.py +++ b/tests/test_tools_platform.py @@ -798,7 +798,6 @@ def annotations(self, name: str) -> ToolAnnotations: ("platform_dataset_info", False), ("platform_graphql_list_types", True), ("platform_graphql_get_types", True), - ("platform_dataset_lookup", True), ], ) def test_read_only(self, name: str, open_world: bool): @@ -807,8 +806,10 @@ def test_read_only(self, name: str, open_world: bool): assert annotations.destructiveHint is False assert annotations.openWorldHint is open_world - def test_dataset_export_writes_but_destroys_nothing(self): - annotations = self.annotations("platform_dataset_export") + @pytest.mark.parametrize("name", ["platform_dataset_export", "platform_dataset_lookup"]) + def test_dataset_tools_write_but_destroy_nothing(self, name: str): + # Both download a completed export into the downloads directory. + annotations = self.annotations(name) assert annotations.readOnlyHint is False assert annotations.destructiveHint is False assert annotations.idempotentHint is False From cdcfde339b8ad4074b4b524cd7b2649ef617f5e1 Mon Sep 17 00:00:00 2001 From: John Anderson Date: Wed, 7 Oct 2026 15:28:28 -0400 Subject: [PATCH 16/16] fix: annotate dataset lookup as writing, and keep large export writes off the event loop --- stacklet/mcp/platform/graphql.py | 2 +- stacklet/mcp/platform/tools.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/stacklet/mcp/platform/graphql.py b/stacklet/mcp/platform/graphql.py index 09b3a2e..54e89b3 100644 --- a/stacklet/mcp/platform/graphql.py +++ b/stacklet/mcp/platform/graphql.py @@ -304,7 +304,7 @@ async def _save_stream(response: httpx.Response) -> str: try: with f: async for chunk in response.aiter_bytes(): - f.write(chunk) + await asyncio.to_thread(f.write, chunk) except BaseException: os.unlink(f.name) raise diff --git a/stacklet/mcp/platform/tools.py b/stacklet/mcp/platform/tools.py index 749badc..ad40abd 100644 --- a/stacklet/mcp/platform/tools.py +++ b/stacklet/mcp/platform/tools.py @@ -28,7 +28,7 @@ HOSTED_DOWNLOAD_NOTE = ( "Open this link in a browser where you're signed in to Stacklet. " - "It works only for you, until available_until." + "It works only for you, until the export expires." ) EXPIRED_DOWNLOAD_NOTE = "This export has expired. Start a new one with platform_dataset_export." @@ -58,7 +58,7 @@ def tools() -> list[Tool]: # Exports don't change governance data, but they do start a job and # write a file server-side, and each call starts another one. make_tool(platform_dataset_export, read_only=False, destructive=False, idempotent=False), - make_tool(platform_dataset_lookup, read_only=True), + make_tool(platform_dataset_lookup, read_only=False, destructive=False, idempotent=False), ]