Skip to content

fix: bound hosted binary tool responses - #112

Merged
ChiragAgg5k merged 2 commits into
mainfrom
fix/bound-hosted-binary-responses
Aug 31, 2026
Merged

fix: bound hosted binary tool responses#112
ChiragAgg5k merged 2 commits into
mainfrom
fix/bound-hosted-binary-responses

Conversation

@ChiragAgg5k

Copy link
Copy Markdown
Member

Summary

  • stream hosted binary Appwrite responses into a 25 MiB bounded buffer instead of letting the generated SDK buffer them without a limit
  • reject oversized responses from either Content-Length or observed streamed bytes with a stable hosted_response_too_large MCP tool error
  • request identity encoding and reject unexpected compressed responses before decompression can bypass the limit
  • preserve the existing generated-SDK path for self-hosted stdio deployments
  • document the hosted limit in binary tool descriptions and classify limit rejections separately in telemetry
  • preserve Appwrite-formatted errors for HTTP status and transport failures

Context

Production pods were OOM-killed while appwrite_call_tool was inflight. Binary SDK methods currently read the full body into memory, after which MCP base64 and JSON serialization create additional copies. A single large response can therefore exceed the 1 GiB pod limit.

This keeps binary methods available and backward-compatible for responses up to 25 MiB. Larger responses fail predictably and direct clients to an Appwrite SDK or REST API.

Validation

  • uv run --group dev ruff check src tests
  • uv run --group dev black --check src tests
  • uv run --group dev pyright
  • uv run python -m unittest discover -s tests/unit -v (252 tests)

@greptile-apps

greptile-apps Bot commented Aug 31, 2026

Copy link
Copy Markdown

Greptile Summary

The PR bounds hosted binary Appwrite responses at 25 MiB while preserving the generated SDK path for local stdio deployments. The compressed-error fix prevents decompression before bounding, but currently bypasses Appwrite error translation for compressed HTTP failures.

  • Adds streamed byte and Content-Length limits for hosted binary tools.
  • Rejects non-identity response encodings before body iteration.
  • Adds a stable oversized-response category, tool documentation, and unit coverage.

Confidence Score: 4/5

The PR should not merge until compressed upstream HTTP errors retain the existing Appwrite error contract.

The reordered encoding guard protects memory but converts compressed 4xx and 5xx responses into generic ValueErrors before their status, type, body, and telemetry classification can be preserved.

Files Needing Attention: src/mcp_server_appwrite/server.py

Important Files Changed

Filename Overview
src/mcp_server_appwrite/server.py Adds hosted bounded binary streaming, but compressed HTTP errors now bypass Appwrite status and error formatting.
src/mcp_server_appwrite/error_classification.py Adds a structured oversized-response exception and dedicated telemetry category.
src/mcp_server_appwrite/service.py Adds the configured hosted binary limit to binary tool descriptions.
src/mcp_server_appwrite/constants.py Defines the 25 MiB hosted binary source-byte limit.
tests/unit/test_server.py Covers bounded streaming and compressed-response rejection, while codifying the generic ValueError behavior for compressed HTTP errors.

Fix all with Greploop Fix All in Claude Code Fix All in Codex

Prompt To Fix All With AI
### Issue 1
src/mcp_server_appwrite/server.py:887-891
**Compressed errors lose Appwrite context**

If an upstream server or intermediary returns a compressed HTTP 4xx or 5xx response despite the identity request, this guard raises `ValueError` before `_raise_bounded_response_error` can translate the status. The caller consequently loses the upstream status, type, and body, while telemetry loses the `appwrite_4xx` or `appwrite_5xx` classification and associated 5xx monitoring.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Reviews (2): Last reviewed commit: "fix: guard compressed upstream error bod..." | Re-trigger Greptile

Comment thread src/mcp_server_appwrite/server.py Outdated
Comment on lines +887 to +891
if content_encoding.lower().strip() not in {"", "identity"}:
raise ValueError(
"Hosted MCP cannot safely return a compressed binary response. "
"Use an Appwrite SDK or REST API for this content."
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Compressed errors lose Appwrite context

If an upstream server or intermediary returns a compressed HTTP 4xx or 5xx response despite the identity request, this guard raises ValueError before _raise_bounded_response_error can translate the status. The caller consequently loses the upstream status, type, and body, while telemetry loses the appwrite_4xx or appwrite_5xx classification and associated 5xx monitoring.

Knowledge Base Used: Observability and error handling

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/mcp_server_appwrite/server.py
Line: 887-891

Comment:
**Compressed errors lose Appwrite context**

If an upstream server or intermediary returns a compressed HTTP 4xx or 5xx response despite the identity request, this guard raises `ValueError` before `_raise_bounded_response_error` can translate the status. The caller consequently loses the upstream status, type, and body, while telemetry loses the `appwrite_4xx` or `appwrite_5xx` classification and associated 5xx monitoring.

**Knowledge Base Used:** [Observability and error handling](https://app.greptile.com/appwrite/-/custom-context/knowledge-base/appwrite/mcp/-/docs/observability-and-error-handling.md)

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Fix in Claude Code Fix in Codex

@ChiragAgg5k
ChiragAgg5k merged commit 0ec6972 into main Aug 31, 2026
5 checks passed
@ChiragAgg5k
ChiragAgg5k deleted the fix/bound-hosted-binary-responses branch August 31, 2026 13:40
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