From 3108255530e26898f86507865407bf25bbb996ad Mon Sep 17 00:00:00 2001 From: Doron Shalev Date: Tue, 8 Sep 2026 21:05:08 +0300 Subject: [PATCH 1/3] fix: describe a draft data asset's files as readable `get_data_asset_file_urls` told the model to call it only on an asset in a ready state, and to poll `wait_until_ready` first otherwise. A draft data asset never reaches a ready state, so a model that follows this on a draft waits for something that cannot happen: half an hour, now that tool calls are bounded, which is a hung turn from the user's side. The Aqua upload flow hands the agent a draft and asks it to read the files, so this is the common case, not a corner. Say what is true instead: a draft's files are readable as they are, and readiness only concerns an asset still being created by a copy-based path, so waiting stays available where waiting is right. `wait_until_ready` picks up the same point from the other side, since its own description offered downloading files as the reason to poll. The appended guidance also ran into the SDK docstring's final word, because the docstring ends without trailing whitespace. The new test reads the composed description the server advertises rather than the source literal, so it covers the join as well as the wording. Co-Authored-By: Claude Opus 5 (1M context) --- src/codeocean_mcp_server/tools/data_assets.py | 10 +++--- tests/test_tool_descriptions.py | 33 +++++++++++++++++++ 2 files changed, 38 insertions(+), 5 deletions(-) create mode 100644 tests/test_tool_descriptions.py diff --git a/src/codeocean_mcp_server/tools/data_assets.py b/src/codeocean_mcp_server/tools/data_assets.py index fc8f9da..a480b43 100644 --- a/src/codeocean_mcp_server/tools/data_assets.py +++ b/src/codeocean_mcp_server/tools/data_assets.py @@ -46,10 +46,9 @@ def get_data_asset(data_asset_id: str) -> DataAsset: @mcp.tool( description=( str(client.data_assets.get_data_asset_file_urls.__doc__) - + "Call only when the data asset is already created and in a ready " - "state. If the asset may not yet be ready, first use " - "`wait_until_ready` to poll until readiness, then retrieve the " - "download URL." + + " A draft data asset's files are readable right away: a draft never becomes ready, so never wait on " + "one. Only an asset still being created by a copy-based path (captured result, connector, import) may " + "need `wait_until_ready` first." ) ) def get_data_asset_file_urls(data_asset_id: str, file_path: str) -> FileURLs: @@ -81,7 +80,8 @@ def update_metadata( str(client.data_assets.wait_until_ready.__doc__) + "Poll until the specified data asset becomes ready before " "performing further operations (e.g., downloading files). You can " - "set `polling_interval` and optional `timeout`." + "set `polling_interval` and optional `timeout`. Never call this on a " + "draft: a draft never becomes ready, and its files are readable as they are." ) ) def wait_until_ready( diff --git a/tests/test_tool_descriptions.py b/tests/test_tool_descriptions.py new file mode 100644 index 0000000..5514525 --- /dev/null +++ b/tests/test_tool_descriptions.py @@ -0,0 +1,33 @@ +"""Tests for the tool descriptions the server advertises. + +Descriptions are composed by concatenating the SDK docstring with the server's own guidance, so +these assertions read the composed text a model actually receives rather than the source literals. +""" + +from mcp_client import get_tools + +DESCRIPTIONS = {tool.name: tool.description or "" for tool in get_tools()} + + +def test_file_urls_description_joins_the_sdk_docstring_with_a_space(): + """The appended guidance does not run into the last word of the SDK docstring.""" + assert "data asset. A draft" in DESCRIPTIONS["get_data_asset_file_urls"] + + +def test_file_urls_description_presents_a_draft_as_readable(): + """A draft never becomes ready, so the description must not send the model off to wait on one.""" + description = DESCRIPTIONS["get_data_asset_file_urls"] + assert "a draft never becomes ready, so never wait on one" in description + assert "already created and in a ready state" not in description + + +def test_file_urls_description_keeps_readiness_for_copy_based_paths(): + """Waiting is still the right move for an asset that is genuinely still being created.""" + description = DESCRIPTIONS["get_data_asset_file_urls"] + assert "copy-based path (captured result, connector, import)" in description + assert "`wait_until_ready`" in description + + +def test_wait_until_ready_description_rules_out_drafts(): + """The polling tool itself says a draft is not something to poll.""" + assert "Never call this on a draft" in DESCRIPTIONS["wait_until_ready"] From 94784bb59c85d665fea7e5b19586ec82c8258123 Mon Sep 17 00:00:00 2001 From: Doron Shalev Date: Tue, 8 Sep 2026 21:09:50 +0300 Subject: [PATCH 2/3] test: assert the description join without the SDK's wording The spacing test matched "data asset. A draft", which would fail on an upstream docstring reword even though the server-side join was still right. It now finds the appended guidance and checks that what precedes it ends in whitespace, so only the join is under test. Dropping the leading space in the source still fails it. Co-Authored-By: Claude Opus 5 (1M context) --- tests/test_tool_descriptions.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/tests/test_tool_descriptions.py b/tests/test_tool_descriptions.py index 5514525..8c477a3 100644 --- a/tests/test_tool_descriptions.py +++ b/tests/test_tool_descriptions.py @@ -8,10 +8,15 @@ DESCRIPTIONS = {tool.name: tool.description or "" for tool in get_tools()} +# The first words the server appends to the SDK docstring for get_data_asset_file_urls. +FILE_URLS_GUIDANCE = "A draft data asset's files are readable right away" -def test_file_urls_description_joins_the_sdk_docstring_with_a_space(): + +def test_file_urls_description_separates_the_guidance_from_the_sdk_docstring(): """The appended guidance does not run into the last word of the SDK docstring.""" - assert "data asset. A draft" in DESCRIPTIONS["get_data_asset_file_urls"] + description = DESCRIPTIONS["get_data_asset_file_urls"] + assert FILE_URLS_GUIDANCE in description + assert description[: description.index(FILE_URLS_GUIDANCE)].endswith((" ", "\n")) def test_file_urls_description_presents_a_draft_as_readable(): From c76b9429c324e6a865bf009e80dd2dd2d27e287b Mon Sep 17 00:00:00 2001 From: Doron Shalev Date: Tue, 8 Sep 2026 21:16:58 +0300 Subject: [PATCH 3/3] fix: separate get_capsule's guidance from its docstring Generalising the join test to every advertised description turned up one more run-on: `get_capsule` served "...capsule by its ID.Use only to fetch metadata ...", the same missing separator as `get_data_asset_file_urls`. The test now discovers the joins instead of naming one. It walks the SDK client classes for docstrings, finds every tool description that is one of those docstrings plus appended text, and asserts the boundary carries whitespace, so a tool gets covered the moment someone appends guidance to it. A scan guard fails if the discovery matches nothing, since a silent zero-match would make the boundary assertion vacuous. `wait_until_ready` and `create_data_asset` omit a separator of their own but are not run-ons: their SDK docstrings end in "\n" and "\n\n", so the composed text breaks at a newline. Left as they are, because prepending a space would put a stray space at the start of a line, and forcing a single space would glue the guidance onto the last entry of a `Raises:` block. The boundary test covers what actually matters, and covers it against an upstream reword too. Co-Authored-By: Claude Opus 5 (1M context) --- src/codeocean_mcp_server/tools/capsules.py | 2 +- tests/test_tool_descriptions.py | 68 +++++++++++++++++++--- 2 files changed, 60 insertions(+), 10 deletions(-) diff --git a/src/codeocean_mcp_server/tools/capsules.py b/src/codeocean_mcp_server/tools/capsules.py index 195e4e0..7b04f5f 100644 --- a/src/codeocean_mcp_server/tools/capsules.py +++ b/src/codeocean_mcp_server/tools/capsules.py @@ -42,7 +42,7 @@ def search_pipelines( @mcp.tool( description=( - str(client.capsules.get_capsule.__doc__) + "Use only to fetch metadata for a known capsule ID. " + str(client.capsules.get_capsule.__doc__) + " Use only to fetch metadata for a known capsule ID. " "Do not use for searching." ) ) diff --git a/tests/test_tool_descriptions.py b/tests/test_tool_descriptions.py index 8c477a3..dc40ab7 100644 --- a/tests/test_tool_descriptions.py +++ b/tests/test_tool_descriptions.py @@ -1,22 +1,72 @@ """Tests for the tool descriptions the server advertises. -Descriptions are composed by concatenating the SDK docstring with the server's own guidance, so -these assertions read the composed text a model actually receives rather than the source literals. +Descriptions are composed by concatenating an SDK docstring with the server's own guidance, so these +assertions read the composed text a model actually receives rather than the source literals. """ +import inspect + +import pytest +from codeocean.capsule import Capsules +from codeocean.computation import Computations +from codeocean.custom_metadata import CustomMetadata +from codeocean.data_asset import DataAssets +from codeocean.pipeline import Pipelines from mcp_client import get_tools DESCRIPTIONS = {tool.name: tool.description or "" for tool in get_tools()} -# The first words the server appends to the SDK docstring for get_data_asset_file_urls. -FILE_URLS_GUIDANCE = "A draft data asset's files are readable right away" +# The SDK clients whose docstrings the tool descriptions are built from. +SDK_CLIENTS = (Capsules, Computations, CustomMetadata, DataAssets, Pipelines) -def test_file_urls_description_separates_the_guidance_from_the_sdk_docstring(): - """The appended guidance does not run into the last word of the SDK docstring.""" - description = DESCRIPTIONS["get_data_asset_file_urls"] - assert FILE_URLS_GUIDANCE in description - assert description[: description.index(FILE_URLS_GUIDANCE)].endswith((" ", "\n")) +def _sdk_docstrings(): + """Yield (qualified name, docstring) for every documented method of the SDK clients.""" + for client in SDK_CLIENTS: + for name, method in inspect.getmembers(client, inspect.isfunction): + if method.__doc__ and method.__doc__.strip(): + yield f"{client.__name__}.{name}", method.__doc__ + + +def _appended_descriptions(): + """Find every advertised description that is an SDK docstring followed by server-injected text. + + Returns a (tool, SDK method, appended remainder) triple per join, so a failure names the join + rather than just the tool. + """ + found = [] + for tool, description in DESCRIPTIONS.items(): + for source, docstring in _sdk_docstrings(): + body = docstring.rstrip() + if description.startswith(body) and len(description) > len(body): + found.append((tool, source, description[len(body) :])) + return found + + +APPENDED_DESCRIPTIONS = _appended_descriptions() + + +def test_the_scan_finds_the_composed_descriptions(): + """Guard the scan itself: were it to match nothing, the boundary test below would be vacuous.""" + tools = {tool for tool, _, _ in APPENDED_DESCRIPTIONS} + assert {"get_data_asset_file_urls", "wait_until_ready", "create_data_asset", "get_capsule"} <= tools + + +@pytest.mark.parametrize( + ("tool", "source", "appended"), + APPENDED_DESCRIPTIONS, + ids=[f"{tool}-{source}" for tool, source, _ in APPENDED_DESCRIPTIONS], +) +def test_appended_guidance_is_separated_from_the_sdk_docstring(tool, source, appended): + """Server guidance never runs into the docstring's final word. + + Some SDK docstrings end in a newline and some do not, so a join that omits its own separator is + correct only by the grace of upstream's whitespace. Asserting on the composed text catches both + a missing separator here and a docstring reworded upstream. + """ + assert appended[:1].isspace(), ( + f"{tool} appends server guidance straight onto the last word of {source}'s docstring: ...{appended[:40]!r}" + ) def test_file_urls_description_presents_a_draft_as_readable():