Skip to content

feat(mcp): centralise ToolAnnotations across public + SDK tool surface - #87

Open
louzt wants to merge 4 commits into
appwrite:mainfrom
louzt:feat/mcp-tool-annotations
Open

feat(mcp): centralise ToolAnnotations across public + SDK tool surface#87
louzt wants to merge 4 commits into
appwrite:mainfrom
louzt:feat/mcp-tool-annotations

Conversation

@louzt

@louzt louzt commented Jul 27, 2026

Copy link
Copy Markdown

Summary

Adds MCP ToolAnnotations (per MCP 2025-06-18 spec) to every tool the
server can emit — the 3 public operator tools, the optional docs search
tool, AND every method on every Appwrite SDK service generated
dynamically by service.py. Driven by a single helper module so the
hints stay in lock-step with the action verb in each tool name.

Closes/updates #86.

What's in the diff

New module — src/mcp_server_appwrite/annotations.py

Three pure helpers that form the single source of truth for safety
hints:

  • classify_tool_name(tool_name) — extract the action verb and bucket
    the tool as read / write / delete / unknown.
  • annotations_for_classification(bucket) — return a canonical
    ToolAnnotations instance with every hint set explicitly. The inline
    rationale comments document why each hint has its value (e.g. why
    unknown.openWorldHint=True despite not knowing the verb, why
    delete.idempotentHint=True).
  • annotations_for_gateway() — dedicated helper for the dispatcher
    tool (appwrite_call_tool); leaves destructiveHint=None so the
    MCP 2025-06-18 spec defaults it to True and clients that gate
    human approval on the hint correctly prompt the user for every
    gateway call. See Review feedback resolution below for the
    rationale.

service.py — 25 SDK services, hundreds of methods

Service.list_tools() now classifies each SDK method by its action verb
and attaches the resulting annotations. This covers every method on
every service the Appwrite SDK ships.

operator.py — 3 public tools

appwrite_get_context, appwrite_search_tools, appwrite_call_tool
now use annotations_for_classification (or annotations_for_gateway
for the dispatcher) instead of inline ToolAnnotations(...)
constructions. Old inline blocks were removed in the same commit.

docs_search.py — 1 docs tool

appwrite_search_docs uses the helper for its read annotation.

Why a single helper instead of inline annotations per tool

  • 30 tools × 4 hints = 120 places where drift can creep in. One bug in
    classify_tool_name would silently mislead every MCP client that
    filters on these hints (Claude Code, Gemini MCP, Appwrite broker).
  • Pure functions make the mapping exhaustively testable.
  • Future SDK methods automatically get the right hints without anyone
    having to remember the pattern.

Test coverage expansion (from 13 → 41 net new tests)

  • classify_tool_name parametrized over 19 read verbs, 19 write verbs,
    10 delete verbs, 4 unknown names, case-insensitivity, verbs in the
    middle of names.
  • annotations_for_classification for every bucket; explicit-field check
    catches MCP-default leakage; unrecognised inputs fall through to
    unknown.
  • annotations_for_gateway for the dispatcher tool: contract pinned
    via 5 dedicated tests (destructiveHint is None, other hints
    explicit, distinct from unknown, fresh instance per call,
    title is None).
  • Parity with operator._classify_verb for every known verb × resource
    combination — both helpers stay in lock-step.
  • Public tools: appwrite_get_context / appwrite_search_tools are
    read; appwrite_call_tool is the gateway (destructiveHint=None);
    the docs search tool is read.
  • Dynamic SDK tools: Service.list_tools() is invoked on a stub users
    service and the resulting Tool objects carry annotations consistent
    with the verb bucket.

The "consistent with" check accounts for the MCP spec limitation that
write and unknown collapse to the same wire shape on the four hints.

Total: 210 unit tests pass locally (post upstream
appwrite/mcp#84 — Appwrite
Python SDK 22.2.0 upgrade, which added 83 net new method-level
tests). All four CI jobs (ruff, black, pyright, unittest) green.

Rationale

Per the MCP 2025-06-18 spec, leaving a hint unset means unknown and
forces conservative client behavior (user prompt before execution). With
explicit annotations, clients can:

  • Auto-execute read-only context/search/SDK-list calls (no prompt)
  • Keep gating write/delete SDK calls on the existing
    confirm_write=true mechanism inside appwrite_call_tool (the
    destructive gate lives in input validation, not at the MCP hint layer)

The annotation for appwrite_call_tool is destructiveHint=None
(unset), not False. An unset hint makes the gateway honest about
its dispatcher semantics: it cannot promise either "non-destructive"
or "destructive" statically, because the actual capability depends on
the sub-tool chosen at runtime. Per the MCP 2025-06-18 spec, an unset
destructiveHint defaults to True for clients that gate human
approval on the hint — which matches the runtime
confirm_write=true boundary enforced inside _call_hidden_tool. See
Review feedback resolution below for the full decision matrix and
the Greptile P1 finding that drove this design.

Scope Boundary

This PR covers every tool the server can advertise:

  • 3 public operator tools (appwrite_get_context,
    appwrite_search_tools, appwrite_call_tool)
  • 1 optional docs search tool (appwrite_search_docs)
  • ~25 SDK services × multiple methods (every method emitted by
    service.Service.list_tools())

It does NOT cover:

  • Tool-level OAuth/RBAC hints (scopes, permissions) — these are
    not part of the MCP ToolAnnotations schema.
  • Per-call confirmation UX changes — those belong to the client, not
    the server.

Validation

$ uv run --group dev ruff check src tests
All checks passed!

$ uv run --group dev black --check src tests
40 files would be left unchanged.

$ uv run --group dev pyright
0 errors, 0 warnings, 0 informations

$ uv run python -m unittest discover -s tests/unit
Ran 210 tests in 2.822s
OK

5 files changed, +744/-1 against appwrite/mcp#84 (the SDK 22.2.0
upgrade added ~83 method-level test cases; the F401 fix in the rebase
commit removes one line).

Compatibility

Tested against the MCP 2025-06-18 spec for ToolAnnotations. Clients
that ignore the annotations (older MCP implementations) keep working
unchanged — annotations are hints, not gates. Clients that honor them
gain the auto-execute benefit on read-only tools.

Related

Rebase onto upstream PR #84 (SDK 22.2.0)

This branch was rebased onto
appwrite/mcp#84 (merged
2026-07-27) before the most recent push, so it sits cleanly on top of
the Appwrite Python SDK 22.2.0 upgrade. The rebase auto-merged one
file (src/mcp_server_appwrite/docs_search.py) where my commit
9f443b6 had added an import that upstream commit
8a962e4 ("Stop emitting metrics the MCP dashboard no longer queries")
had independently removed. The conflict was resolved by keeping my
annotations-related imports and dropping the now-unused
from . import telemetry import in a separate, focused commit so the
lint job (ruff rule F401) stays green.

The four commits in the branch (all SSH-signed by
louzt <davidmirelesll@outlook.com>):

SHA Subject
256f905 feat(mcp): add safety ToolAnnotations to public tools
76ae829 feat(mcp): centralise ToolAnnotations across public + SDK tool surface
44463b2 feat(mcp): split gateway annotations so destructiveHint stays unset (PR #87 review)
42385cf fix(rebase): drop orphaned 'from . import telemetry' after SDK 22.2.0 rebase

Review feedback resolution

Greptile Bot reviewed this PR and reported the following issue, which is
fixed in the head commit 42385cf:

Source Severity File Resolution
Greptile comment P1 src/mcp_server_appwrite/operator.py:263 appwrite_call_tool advertised destructiveHint=False while dispatching delete-classified SDK methods when given confirm_write=true.

Fix — split gateway annotations from the verb-bucket helpers

The annotations helper now exposes a separate, dedicated
annotations_for_gateway() function. appwrite_call_tool uses it
instead of falling through to the "unknown" classification bucket.

# src/mcp_server_appwrite/annotations.py
def annotations_for_gateway() -> types.ToolAnnotations:
    return types.ToolAnnotations(
        title=None,
        readOnlyHint=False,
        # UNSET on purpose: the MCP 2025-06-18 spec defaults an unset
        # `destructiveHint` to True. Clients that gate human approval on
        # the hint (Claude Code, Gemini MCP) therefore prompt the user
        # for every gateway call — matching the runtime
        # `confirm_write=true` boundary inside `_call_hidden_tool`.
        destructiveHint=None,
        idempotentHint=False,
        openWorldHint=True,
    )

Why destructiveHint=None and not False

Hint value Semantics Right for the gateway?
False "I promise this tool is non-destructive." No — a false promise; the gateway can dispatch delete.
True "I promise this tool is destructive." No — a false promise; the gateway can dispatch reads.
None (unset) "I cannot promise anything static; apply host policy." Yes — honest answer for a dynamic dispatcher.

The MCP spec defaults None to True, so the practical effect on
clients that ignore None is identical to explicitly setting True.
Clients that distinguish the three (some implementations surface
"unknown" to the user explicitly) get the most accurate signal.

Regression coverage

  • tests/unit/test_annotations.py:AnnotationsForGatewayTests — 5 new
    tests pinning the gateway contract (destructiveHint is None,
    other hints explicit, distinct from unknown, fresh instance per
    call, title is None).
  • tests/unit/test_annotations.py:test_appwrite_call_tool_is_gateway
    — replaces the prior _is_unknown test and pins the live contract
    with a regression-guard comment naming this exact Greptile review.

Verification

CI on 42385cf (post rebase onto appwrite/mcp#84):
  ruff     ✓ All checks passed!
  black    ✓ 40 files unchanged
  pyright  ✓ 0 errors, 0 warnings
  unittest ✓ 210 tests, OK
  mergeable ✓ MERGEABLE

Summary by CodeRabbit

  • New Features

    • Added safety and behavior annotations to Appwrite tools.
    • Tools are now classified as read, write, delete, or unknown.
    • Public gateway and documentation-search tools expose appropriate read-only or conservative behavior hints.
    • Dynamically generated tools include annotations describing whether actions may modify or delete data.
  • Tests

    • Added coverage for annotation classification and tool behavior across supported tool types.

@greptile-apps

greptile-apps Bot commented Jul 27, 2026

Copy link
Copy Markdown

Greptile Summary

Centralizes MCP tool safety annotations across the public, documentation-search, and dynamically generated SDK tool surfaces.

  • Adds shared helpers for classifying tool names and constructing canonical annotations.
  • Gives the dynamic gateway a conservative annotation profile with its destructive hint unset.
  • Adds annotation coverage for public tools, documentation search, SDK services, and gateway behavior.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains; the gateway no longer promises non-destructive behavior and the prior finding is fixed.

Important Files Changed

Filename Overview
src/mcp_server_appwrite/annotations.py Defines centralized classification and annotation helpers, including a conservative gateway profile that resolves the previously reported destructive-capability mismatch.
src/mcp_server_appwrite/operator.py Applies read annotations to non-mutating public tools and the dedicated gateway annotations to appwrite_call_tool.
src/mcp_server_appwrite/service.py Adds verb-derived annotations to dynamically generated Appwrite SDK tools.
src/mcp_server_appwrite/docs_search.py Marks documentation search as read-only while retaining its open-world network interaction hint.
tests/unit/test_annotations.py Adds broad regression coverage for classification, annotation mappings, generated tools, public tools, and the gateway fix.

Reviews (3): Last reviewed commit: "fix(rebase): drop orphaned 'from . impor..." | Re-trigger Greptile

Comment thread src/mcp_server_appwrite/operator.py
louzt added a commit to louzt/appwrite-mcp that referenced this pull request Jul 27, 2026
appwrite#87 review)

Greptile flagged appwrite#87 operator.py:263 as P1: the
appwrite_call_tool gateway dispatched to delete-classified SDK
methods while advertising destructiveHint=False, letting clients
that gate human approval on the hint treat a destructive tool as
non-destructive.

Fix: introduce annotations_for_gateway() in annotations.py.
destructiveHint is intentionally set to None (unset) — the MCP
2025-06-18 spec defaults an unset hint to True, so clients default
to destructive and prompt the user for every gateway call. This
matches the runtime confirm_write=true boundary enforced inside
_call_hidden_tool, but at the layer where MCP clients (Claude Code,
Gemini MCP, Appwrite broker) actually make the approval decision.

annotations_for_classification("unknown") is preserved for verb
buckets we genuinely cannot derive; the gateway's profile is
intentionally separate because the security semantics differ
(unknown cannot promise non-destructive; gateway cannot promise
anything static).

- 5 new tests in AnnotationsForGatewayTests
- test_appwrite_call_tool_is_unknown renamed and updated to assert
  destructiveHint is None (with a regression-guard comment)
- AnnotationsModuleSurfaceTests now covers the new public name

CI: ruff ✓, black ✓, pyright ✓, unittest 127/127 ✓.
louzt added 4 commits July 27, 2026 12:17
Adds ToolAnnotations (readOnlyHint, destructiveHint, idempotentHint,
openWorldHint, title) to the 4 public MCP tools exposed by Appwrite MCP
per AGENTS.md §Tool surface:

- appwrite_get_context: readOnly, idempotent, openWorld (calls Cloud APIs)
- appwrite_search_tools: readOnly, idempotent, NOT openWorld (local catalog)
- appwrite_call_tool: NOT readOnly, NOT idempotent, openWorld (dispatches
  to Appwrite SDK with confirm_write=true gate on mutating calls)
- appwrite_search_docs: readOnly, idempotent, NOT openWorld (local index)

Each hint carries an inline comment explaining the semantic rationale,
matching the pattern already established in serpapi-mcp-fork so
reviewers can audit the readOnly/destructive/idempotent/openWorld
choices at a glance.

Includes tests/unit/test_annotations.py with 13 unittest-style tests
covering: tool surface count + uniqueness, every-hint-is-bool shape,
readOnly implies not-destructive, readOnly implies idempotent,
appwrite_call_tool honest semantics, search tools are not open world,
and human-readable title presence. All 111 unit tests pass.

Per the MCP 2025-06-18 spec, leaving a hint unset means 'unknown' and
forces clients to prompt the user. With these annotations, MCP clients
(Claude Code, Cursor, Antigravity) can safely auto-execute the
read-only context and search tools while keeping write/delete calls
gated on the existing confirm_write=true mechanism.
Expand the ToolAnnotations work from 4 public tools to cover every tool the
server can emit (4 public + ~25 dynamic SDK services + 1 docs tool), driven
by a single helper module so the hints stay in lock-step with the action
verb in the tool name.

What's in the diff:

- New module ``src/mcp_server_appwrite/annotations.py`` exporting two pure
  helpers:

  - ``classify_tool_name(tool_name)`` — extract the action verb and bucket
    the tool as ``read`` / ``write`` / ``delete`` / ``unknown``.
  - ``annotations_for_classification(bucket)`` — return a canonical
    ``ToolAnnotations`` instance with every hint set explicitly. The inline
    rationale comments document why each hint has its value (e.g. why
    ``unknown.openWorldHint=True`` despite not knowing the verb, why
    ``delete.idempotentHint=True``).

- ``service.py``: ``Service.list_tools()`` now classifies each SDK method
  by its action verb and attaches the resulting annotations. This covers
  every method on every service the Appwrite SDK ships (~25 services,
  hundreds of methods).

- ``operator.py``: the 3 public tools now use ``annotations_for_classification``
  instead of inline ``ToolAnnotations(...)`` constructions. The old inline
  blocks were removed in the same commit to keep the diff reviewable.

- ``docs_search.py``: the docs search tool uses the helper for its
  ``read`` annotation.

Why a single helper instead of inline annotations per tool:

- 30 tools × 4 hints = 120 places where drift can creep in. One bug in
  ``classify_tool_name`` would silently mislead every MCP client that
  filters on these hints (claude-code, Gemini MCP, Appwrite broker).
- Pure functions make the mapping exhaustively testable.
- Future SDK methods automatically get the right hints without anyone
  having to remember the pattern.

Test coverage expansion (from 13 → 41 net new tests):

- ``classify_tool_name`` parametrized over 19 read verbs, 19 write verbs,
  10 delete verbs, 4 unknown names, case-insensitivity, verbs in the
  middle of names.
- ``annotations_for_classification`` for every bucket; explicit-field check
  catches MCP-default leakage; unrecognised inputs fall through to
  ``unknown``.
- Parity with ``operator._classify_verb`` for every known verb × resource
  combination — both helpers stay in lock-step.
- Public tools: ``appwrite_get_context`` / ``appwrite_search_tools`` are
  read; ``appwrite_call_tool`` is the gateway (``unknown``); the docs
  search tool is read.
- Dynamic SDK tools: ``Service.list_tools()`` is invoked on a stub users
  service and the resulting ``Tool`` objects carry annotations consistent
  with the verb bucket.

The "consistent with" check accounts for the MCP spec limitation that
write and unknown collapse to the same wire shape on the four hints.

Total: 122 unit tests pass locally. All four CI jobs (ruff, black,
pyright, unittest) green.

Refs appwrite#86 — supersedes the 4-tool-only draft.
appwrite#87 review)

Greptile flagged appwrite#87 operator.py:263 as P1: the
appwrite_call_tool gateway dispatched to delete-classified SDK
methods while advertising destructiveHint=False, letting clients
that gate human approval on the hint treat a destructive tool as
non-destructive.

Fix: introduce annotations_for_gateway() in annotations.py.
destructiveHint is intentionally set to None (unset) — the MCP
2025-06-18 spec defaults an unset hint to True, so clients default
to destructive and prompt the user for every gateway call. This
matches the runtime confirm_write=true boundary enforced inside
_call_hidden_tool, but at the layer where MCP clients (Claude Code,
Gemini MCP, Appwrite broker) actually make the approval decision.

annotations_for_classification("unknown") is preserved for verb
buckets we genuinely cannot derive; the gateway's profile is
intentionally separate because the security semantics differ
(unknown cannot promise non-destructive; gateway cannot promise
anything static).

- 5 new tests in AnnotationsForGatewayTests
- test_appwrite_call_tool_is_unknown renamed and updated to assert
  destructiveHint is None (with a regression-guard comment)
- AnnotationsModuleSurfaceTests now covers the new public name

CI: ruff ✓, black ✓, pyright ✓, unittest 127/127 ✓.
… rebase

The rebase onto upstream/main (PR appwrite#84 SDK 22.2.0) auto-merged the import
but lost the telemetry.record_search_docs(...) call that my original
9f443b6 commit added. Upstream 8a962e4 ('cleanup: stop emitting metrics
the MCP dashboard no longer queries') had already removed both, so the
import was no longer needed. Removing the orphan fixes ruff F401.

Verified locally on this branch:
- ruff check src tests             ✓ All checks passed!
- black --check src tests          ✓ 40 files unchanged
- pyright                          ✓ 0 errors, 0 warnings, 0 informations
- python -m unittest discover -s tests/unit   ✓ 210 tests, OK
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant