Skip to content

feat(mcp): structured tool output, annotations, and isError across the fleet pattern - #6

Merged
kratsg merged 10 commits into
mainfrom
feat/mcp-interop-structured-output
Sep 18, 2026
Merged

feat(mcp): structured tool output, annotations, and isError across the fleet pattern#6
kratsg merged 10 commits into
mainfrom
feat/mcp-interop-structured-output

Conversation

@kratsg

@kratsg kratsg commented Sep 18, 2026

Copy link
Copy Markdown
Collaborator

Summary

Rolls out the shared MCP interop pattern (SDK currency, structured
content, tool annotations, isError) to servicex-mcp's 10 tools,
following the reviewed pilot in af-filesystem-mcp#6.

  • chore(deps): bump mcp lock to 2.2.0 (was already 2.1.1, which has
    the is_error guard convert_result needs); bump the mypy
    pre-commit hook's hard mcp==2.0.0 pin to 2.2.0; add
    mypy_path = ["src"] to pyproject.toml (without it the mypy hook
    only resolves servicex_mcp.* imports when a src/ file is part of
    the same commit).
  • classify_error/check_write_allowed now return
    CallToolResult(is_error=True) instead of a plain string, keeping
    the existing recovery-hint prose byte-for-byte.
  • All 10 tools return Annotated[CallToolResult, ResultModel] with a
    curated markdown text block and structured_content validated
    against a published outputSchema, per
    mcp/server/mcpserver/utilities/func_metadata.py's escape hatch for
    combining the two.
  • ToolAnnotations per the bucket table: 6 read-only
    (open_world_hint=True, external ServiceX backend),
    servicex_submit_query mutating-non-destructive, and 3 destructive
    (servicex_cancel_transform, servicex_delete_transform,
    servicex_delete_dataset) -- cross-checked against the existing
    read_only lifespan flag that already gates these same 4 tools, so
    the two signals cannot drift apart.
  • A registration-level drift test (every tool must declare annotations
    • an outputSchema) and a wire-level e2e test via
      starlette.testclient.TestClient proving tools/list carries
      annotations/outputSchema and tools/call carries both a text
      block and structuredContent.
  • New ## Available tools section in README.md (this repo had no
    existing tool-list section) with a read/write/destructive column.
  • Patch version bump via tbump (0.1.4 -> 0.1.5), chart
    appVersion/version in lockstep.

Deviations from the pilot / brief

  • Forced anyio bump: this repo's dependency graph resolves a
    forced anyio 4.15.1 (vs. the 4.14.2 the rest of the fleet keeps)
    as part of the mcp 2.2.0 solve. anyio>=4.15 deprecates the
    anyio.abc.BlockingPortal re-export; starlette 1.6.0's own
    testclient.py still uses that alias internally, so every
    wire-level test importing starlette.testclient tripped
    filterwarnings=["error"] on collection. Added a narrowly-scoped
    filterwarnings ignore for that exact upstream message in
    pyproject.toml -- nothing in servicex-mcp touches the deprecated
    alias directly, and no other repo in the fleet needed this.
  • exclude-newer gotcha: this repo's pixi.toml has
    exclude-newer = "7d" (the pilot has none), which hid mcp 2.2.0
    from a plain pixi update. Worked around by temporarily widening
    exclude-newer to a narrow explicit window just past mcp 2.2.0's
    publish date, running the update, then reverting pixi.toml (the
    lock stays pinned once resolved) -- pixi.toml itself has no net
    diff in this PR.
  • classify_error naming: this repo's error-formatting helper is
    named classify_error (not format_error like the pilot); kept the
    existing name per the brief, only changed its return type.
  • No CLAUDE.md exists in this repo (unlike the other four backend
    repos), so step 9 of the brief (update the add-a-tool template) had
    nothing to update. Flagging for Giordon: worth deciding separately
    whether this repo should get a CLAUDE.md matching the fleet
    convention.
  • docs/index.md has its own hand-maintained tools table, not
    transcluded from README.md via --8<-- snippets (unlike
    af-filesystem-mcp, where docs/index.md does transclude README
    sections). Left it untouched per the brief's scope (README.md only);
    it will read as slightly stale (no read/write column) until someone
    updates it too.
  • tbump.toml's github_url still points at kratsg/servicex-mcp
    (pre-org-migration); left untouched as out of scope for this PR.

Test plan

  • pixi run -e py313 test-all (and py311/py312) -- 303 passed, 1
    skipped (the --runslow live-backend suite)
  • pixi run pre-commit --stage manual -- all hooks pass, including
    mypy via the prek hook
  • pixi run pylint -- 10.00/10
  • pixi run -e helm helm-lint / helm-template -- pass with the
    bumped chart version

🤖 Generated with Claude Code

kratsg added 10 commits September 17, 2026 23:01
@kratsg
chore(deps): bump mcp lock to 2.2.0
4e1f23d 
mcp 2.0.0's FuncMetadata.convert_result validates a returned
CallToolResult's structured_content against the tool's output model
unconditionally. mcp>=2.1.1 adds an "and not result.is_error" guard,
which the upcoming Annotated[CallToolResult, Model] structured-output
pattern depends on: an is_error=True result has no structured_content
to validate, and without the guard convert_result raises a
pydantic.ValidationError instead of returning the error result. This
repo's lock was already on mcp 2.1.1 (has the guard); bump to 2.2.0
for consistency with the rest of the fleet.

Also bumps the mypy pre-commit hook's hard mcp==2.0.0 pin to 2.2.0,
and adds mypy_path = ["src"] to pyproject.toml -- without it, the
mypy hook can only resolve servicex_mcp.* imports when a src/ file
happens to be part of the same commit's changed-file list, breaking
on a test-only commit.

This repo's dependency graph pulls a forced anyio 4.15.1 (vs. the
4.14.2 the rest of the fleet keeps) as part of the mcp 2.2.0 solve.
anyio >=4.15 deprecates the anyio.abc.BlockingPortal re-export;
starlette 1.6.0's own testclient.py still uses that alias internally,
so every wire-level test importing starlette.testclient now trips
filterwarnings=["error"] on collection. Added a narrowly-scoped
filterwarnings ignore for that exact upstream message -- nothing in
servicex-mcp touches the deprecated alias directly.

Assisted-by: Claude (Anthropic)
@kratsg
feat(mcp): classify_error and check_write_allowed return is_error Cal…
0097e13 
…lToolResult

Both shared helpers move from returning a plain error string to
CallToolResult(content=[TextContent(...)], is_error=True), keeping the
existing recovery-hint prose byte-for-byte. No structured_content is
set on either: an error result carries no structured payload (mcp
SDK's convert_result only validates structured_content against the
tool's output model when is_error is false).

This is a prerequisite for the per-tool structured-output commits that
follow: check_write_allowed's write-gate is servicex-mcp's own analog
of rucio-mcp's check_write_allowed, and both must speak the same
is_error CallToolResult shape as classify_error before any tool's
return type can change to Annotated[CallToolResult, Model].

Adds a tool_text(result) fixture to tests/conftest.py so existing
markdown assertions keep working once tool functions stop returning
plain strings.

Assisted-by: Claude (Anthropic)
@kratsg
feat(mcp): servicex_info and servicex_list_code_generators return str…
e0587a1 
…uctured content and read-only annotations

Both tools are read-only queries against the external ServiceX server,
so both get ToolAnnotations(read_only_hint=True, open_world_hint=True).
Return type becomes Annotated[CallToolResult, ResultModel] per the mcp
SDK escape hatch for combining a curated markdown text block with a
validated structuredContent payload: ServicexInfoResult (app_version,
capabilities) and ServicexListCodeGeneratorsResult (generators) mirror
the fields each tool already assembles for its markdown rendering.

Assisted-by: Claude (Anthropic)
@kratsg
feat(mcp): dataset tools return structured content and annotations
aa5913d 
servicex_list_datasets/servicex_get_dataset are read-only queries
against the external ServiceX server (read_only_hint=True,
open_world_hint=True). servicex_delete_dataset actually removes a
cached dataset record, so it gets read_only_hint=False,
destructive_hint=True.

DatasetInfo mirrors _dataset_to_dict fields exactly and is reused as
both the per-row model inside ServicexListDatasetsResult and the whole
result of servicex_get_dataset, avoiding a second near-duplicate
model. ServicexDeleteDatasetResult carries the dataset_id and the
"stale" flag delete_dataset returns from the server. The empty-list
early return in servicex_list_datasets now also builds a valid (empty)
structured payload rather than skipping structured_content -- the
tool output schema is unconditional, so every non-error return must
satisfy it.

Assisted-by: Claude (Anthropic)
@kratsg
feat(mcp): servicex_submit_query returns structured content and a mut…
7838ab8 
…ating annotation

servicex_submit_query mutates ServiceX state (submits a real transform
request) but is not destructive -- it never removes or overwrites
anything -- so it gets read_only_hint=False with destructive_hint left
unset. ServicexSubmitQueryResult carries just the request_id, the one
field the tool returns.

Also routes the ValueError raised by _build_dataset_identifier and
ResultFormat parsing through classify_error instead of building an ad
hoc "Error: {exc}" string by hand at the call site: for both current
failure messages, classify_error falls through to the same "Error:
{exc}" text (neither matches any of its recovery-guidance categories),
so this is byte-identical output while collapsing to a single error
factory function per the fleet convention.

Assisted-by: Claude (Anthropic)
@kratsg
feat(mcp): transform tools return structured content and annotations
5f827e7 
servicex_list_transforms/servicex_get_transform_status are read-only
queries against the external ServiceX server (read_only_hint=True,
open_world_hint=True). servicex_cancel_transform and
servicex_delete_transform both act on real running/finished transforms
and cannot be undone, so both get read_only_hint=False,
destructive_hint=True.

TransformInfo mirrors _transform_to_dict fields exactly (including
log_url, which the list view omits from its markdown table via
_TRANSFORM_KEYS but the structured payload still carries) and is
reused as both the per-row model inside ServicexListTransformsResult
and the whole result of servicex_get_transform_status.
ServicexCancelTransformResult/ServicexDeleteTransformResult each carry
just the transform_id acted on. As with servicex_list_datasets, the
empty-list early return in servicex_list_transforms now builds a valid
(empty) structured payload rather than skipping structured_content.

Assisted-by: Claude (Anthropic)
@kratsg
test(mcp): add a registration-level drift guard and a wire-level e2e …
429d7bb 
…test

test_every_tool_declares_annotations_and_output_schema asserts every
registered tool publishes annotations (with read_only_hint set) and an
outputSchema -- the one test that would catch a future tool skipping
the Annotated[CallToolResult, Model] + ToolAnnotations pattern.

TestStdioAppOverTheWire exercises a real JSON-RPC round trip through
the built ASGI app via starlette.testclient.TestClient, since every
existing tool test calls tool.fn directly and never goes through mcp
SDK serialization -- none of them would notice a missing annotations
or outputSchema on the wire. Built with streamable_http_app(json_response=True)
and a base_url with an explicit port to avoid the DNS-rebinding
protection 421 that host="127.0.0.1" alone would trigger.

Assisted-by: Claude (Anthropic)
@kratsg
docs: add README Available tools table
245f656 
The servicex-mcp README had no tool-list section at all (unlike the
other backend repos in this fleet). Adds a plain "Available tools"
table with a read/write column following the other repos table style.
No --8<-- transclusion markers: docs/index.md already carries its own
separately hand-maintained tools table with no snippet reference back
to README, unlike af-filesystem-mcp, where docs/index.md does
transclude README sections -- so there is no existing transclusion
pattern here to extend.

Assisted-by: Claude (Anthropic)
@kratsg
chore: bump version 0.1.4 -> 0.1.5
63af597 
Assisted-by: Claude (Anthropic)
@kratsg
Revert "chore: bump version 0.1.4 -> 0.1.5"
5243723 
This reverts commit 63af597.
@kratsg
kratsg merged commit 5f56adc into main Sep 18, 2026
11 checks passed
@kratsg
kratsg deleted the feat/mcp-interop-structured-output branch September 18, 2026 10:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@kratsg