fix: handle generated SDK input validation - #117
Conversation
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
HarshMN2345
changed the title
|
@greptile-apps Please review the updated implementation at eabc50d. The query-specific MCP guard has been removed; schema-driven validation belongs to sdk-generator#1899 and sdk-for-console-python#10. MCP now recognizes only the explicit local SDK input-validation exception contract. The PR remains draft because the current lockfile still selects SDK 0.6.0; its new regression is dependency-blocked until a fixed SDK is published and pinned. The PR description records the passing 263-test run with the regenerated SDK source overlay and the failing locked-SDK baseline. |
| def test_call_tool_returns_sdk_input_validation_for_unencoded_queries(self): | ||
| client = build_introspection_client() | ||
| manager = register_services(client, profile=API_KEY_PROFILE) | ||
| server = build_mcp_server(build_operator(manager, client), transport="stdio") | ||
| entry = server.get_request_handler("tools/call") | ||
| self.assertIsNotNone(entry) | ||
|
|
||
| async def run_check(): | ||
| ctx = Mock() | ||
| ctx.protocol_version = "2026-07-28" | ||
| ctx.meta = None | ||
| ctx.session.client_params = None | ||
| for queries in ( | ||
| [{"method": "limit", "values": [1]}], | ||
| [ | ||
| '{"method":"limit","values":[1]}', | ||
| {"method": "offset", "values": [1]}, | ||
| ], | ||
| {"method": "limit", "values": [1]}, | ||
| '{"method":"limit","values":[1]}', | ||
| [1], | ||
| ): | ||
| with self.subTest(queries=queries): | ||
| params = types.CallToolRequestParams( | ||
| name="appwrite_call_tool", | ||
| arguments={ | ||
| "tool_name": "tables_db_list_rows", | ||
| "arguments": { | ||
| "database_id": "database-id", | ||
| "table_id": "table-id", | ||
| "queries": queries, | ||
| }, | ||
| }, | ||
| ) | ||
| result = await entry.handler(ctx, params) | ||
|
|
||
| self.assertTrue(result.is_error) | ||
| self.assertIn("type=sdk_input_validation", result.content[0].text) | ||
| self.assertIn("queries", result.content[0].text) | ||
| self.assertIn("string", result.content[0].text) | ||
|
|
||
| with ( | ||
| patch( | ||
| "requests.sessions.Session.request", | ||
| side_effect=AssertionError( | ||
| "Invalid inputs must not send HTTP requests" | ||
| ), |
There was a problem hiding this comment.
The new public-handler regression depends on SDK-side validation that is absent from the currently locked appwrite-console==0.6.0, so the normal test suite fails for every malformed-input case. Until the dependency requirement and lockfile select a published release containing these guards, removing the MCP guard leaves current installations exposed to the original serialization failure and this PR cannot pass CI.
Prompt To Fix With AI
This is a comment left during a code review.
Path: tests/unit/test_server.py
Line: 540-586
Comment:
**Locked SDK Breaks Regression**
The new public-handler regression depends on SDK-side validation that is absent from the currently locked `appwrite-console==0.6.0`, so the normal test suite fails for every malformed-input case. Until the dependency requirement and lockfile select a published release containing these guards, removing the MCP guard leaves current installations exposed to the original serialization failure and this PR cannot pass CI.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
There was a problem hiding this comment.
Agreed; this is the explicit merge blocker recorded at the top of the PR description, and the PR remains draft. The required sequence is to land the schema-driven generator guards in appwrite/sdk-generator#1899, publish the generated SDK from appwrite/sdk-for-console-python#10, then update MCP's requirement and lockfile to that published version. The actual regenerated SDK passes all 263 MCP unit tests through a temporary source overlay, while locked 0.6.0 reproduces exactly this regression in local and GitHub CI. This thread remains unresolved until the real dependency update makes the normal suite pass; the regression will not be weakened or skipped.
1 participant
Draft: blocked on publishing an
appwrite-consolerelease containing sdk-generator#1899 and sdk-for-console-python#10, then updating MCP's dependency requirement and lockfile to that real release. The lockfile still contains 0.6.0, which does not provide these guards. The new public-handler regression therefore fails with the currently locked dependency; this PR is not ready to merge.Passing query objects where the SDK expects a list of strings previously raised
can only concatenate str (not "dict") to str. The generated SDK now validates string-array parameters from their OpenAPI schemas before serialization and raisesAppwriteException(type="sdk_input_validation")with code 0 and no response. MCP returns that readable error and treats this precise local validation contract as an expected caller failure. Server failures, untyped SDK errors, and response-parsing failures remain reportable.The earlier query-specific MCP guard is removed. Tests call the public MCP handler with the real SDK, verify malformed inputs produce an error before HTTP and without a Sentry event, and retain the existing valid-query argument checks for Unicode, empty lists, null, and omission.
Fixes MCP-X.
Validation:
appwrite-console==0.6.0, the new regression fails for all five malformed-input cases; all 262 other tests pass. The transport boundary is blocked, so this check sends no HTTP requests.git diff --check, and the normal Docker build pass. CI unit tests remain dependency-blocked until the published SDK is pinned.