Skip to content

feat(docs): add a public search route for the deprecated docs MCP server - #166

Merged
ChiragAgg5k merged 2 commits into
mainfrom
feat/docs-search-route
Nov 4, 2026
Merged

ChiragAgg5k merged 2 commits into
mainfrom
feat/docs-search-route

Conversation

@ChiragAgg5k

@ChiragAgg5k ChiragAgg5k commented Nov 4, 2026 •

Copy link
Copy Markdown
Member

Why

The deprecated docs MCP server at mcp-for-docs.appwrite.io is being replaced by a much smaller service that keeps its URL and tools for existing clients. Those clients have no OAuth session, so it can't use appwrite_search_docs over the MCP endpoint, and it shouldn't carry its own 35 MB index and OpenAI key.

What

  • POST /docs/search returns {"results": [...]} using the existing DocsSearch.
  • The route is public, like the server it supports, and rate limited per IP at the edge.
  • Each call's cost is bounded: bodies are capped at 16 KiB, queries at 500 characters, and the page count is fixed at 5.
  • DocsSearch.find splits the ranking from the MCP content formatting. The tool's behaviour is unchanged.

Testing

Added DocsSearchRoute to tests/e2e/test_tools.py, against the real app and index with only the OpenAI embedding stubbed: unauthenticated search, fixed page count, invalid and oversized queries, and malformed bodies. Unit suite, tests/e2e/test_tools.py, ruff and black pass locally.

@hansi-codes

hansi-codes Bot commented Nov 4, 2026 •

Copy link
Copy Markdown

🟡 Tier B · Needs changes before merging

The earlier finding about unauthenticated access to paid embeddings remains open; this incremental change only addresses request-body buffering.

This pull request adds a public POST /docs/search endpoint for clients of the deprecated docs MCP server and separates document ranking from MCP response formatting. The route uses the existing search index, fixes the page limit, validates queries, and includes end-to-end coverage for unauthenticated search and invalid requests.

Latest changes: The route now streams incoming request bytes and returns 413 once they exceed 16 KiB, with the documentation and end-to-end test updated to cover that limit.

Verdict New comments Fixed Still open
💬 Commented 0 1 1
Fix with agent prompt
### Issue 1
src/mcp_server_appwrite/http_app.py:606
**Public route leaves paid embeddings open to unlimited requests**

This now mounts an unauthenticated endpoint for every deployment with docs search enabled, and each accepted POST triggers an OpenAI embedding call. The 500-character cap only bounds an individual request; without an authentication or rate limit, anyone can repeatedly call the public route and consume the deployment's paid API quota.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
📂 Walkthrough · 6
File Change
docs/documentation-search.md Documents the public search route and its response and error behavior, including the 16 KiB body limit.
src/mcp_server_appwrite/constants.py Adds the maximum query-length and request-body-size constants.
src/mcp_server_appwrite/docs_search.py Separates document ranking from MCP content formatting.
src/mcp_server_appwrite/http_app.py Adds the public search endpoint, fixed page count, query validation, and streaming body-size rejection.
src/mcp_server_appwrite/operator.py Exposes the configured docs search instance to the HTTP app.
tests/e2e/test_tools.py Covers unauthenticated search, fixed page count, invalid requests, and oversized request bodies.
⏳ Still open from earlier reviews · 1
✅ Fixed since the last review · 1
  • Limit the body size before parsing JSON · src/mcp_server_appwrite/http_app.py:465

Reviewed the commits since 05170b8 · Details · Comment @hansi-codes review to re-run, or mention @hansi-codes with a question.

@hansi-codes hansi-codes Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔵 Tier A · See the inline comments. Summary

Comment thread src/mcp_server_appwrite/http_app.py Resolved

@hansi-codes hansi-codes Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟢 Tier S · Looks good to merge. Summary

@ChiragAgg5k ChiragAgg5k changed the title feat(docs): add a keyed search route for the legacy docs MCP function feat(docs): add a public search route for the legacy docs MCP function Nov 4, 2026
@ChiragAgg5k ChiragAgg5k changed the title feat(docs): add a public search route for the legacy docs MCP function feat(docs): add a public search route for the deprecated docs MCP server Nov 4, 2026

@hansi-codes hansi-codes Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Tier B · See the inline comments. Summary

Comment thread src/mcp_server_appwrite/http_app.py Resolved
methods=["POST"],
)
]
if operator.docs_search is not None

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Public route leaves paid embeddings open to unlimited requests

This now mounts an unauthenticated endpoint for every deployment with docs search enabled, and each accepted POST triggers an OpenAI embedding call. The 500-character cap only bounds an individual request; without an authentication or rate limit, anyone can repeatedly call the public route and consume the deployment's paid API quota.

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/mcp_server_appwrite/http_app.py
Line: 606

Comment:
**Public route leaves paid embeddings open to unlimited requests**

This now mounts an unauthenticated endpoint for every deployment with docs search enabled, and each accepted POST triggers an OpenAI embedding call. The 500-character cap only bounds an individual request; without an authentication or rate limit, anyone can repeatedly call the public route and consume the deployment's paid API quota.

---

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

🟠 Major · security · Reply if this doesn't apply.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Each call is bounded here, and the route is rate limited per IP at the edge before this ships.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Per-IP edge limiting addresses repeated requests from one address, but it does not bound aggregate embedding spend: clients using many IPs can stay below each IP’s limit while continuing to make requests. Every accepted POST calls docs_search.find (route handler), which ranks the query (embedding path). So the per-call bounds and per-IP limit still leave aggregate paid usage unbounded; a global quota or equivalent cap would close that gap.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This does not apply any more: a per-IP rate limit on this path is now live at the edge in front of the hosted server, at 60 requests a minute. It sits outside this repository, so it does not appear in this diff. Together with the 16 KiB body cap, the 500-character query cap and the fixed page count, that bounds what the route can spend. Keeping the route unauthenticated is deliberate, because the deprecated server it supports is also public.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I accept that the edge rule is live. A limit of 60 requests per IP per minute still does not bound aggregate spend: requests from many distinct IPs can each stay within the limit, and each accepted request reaches the embedding call (route, embedding). The request-size and query limits bound individual calls, not their total. The finding remains open unless there is also an aggregate cap or equivalent control.

Clients of the deprecated docs server have no OAuth session, so its replacement cannot use appwrite_search_docs. Sharing the index here means it needs no OpenAI key or 35 MB artifact, and search stays as fresh as the daily refresh. Queries are capped and the page count fixed, so each call costs one small embedding.
@ChiragAgg5k
ChiragAgg5k force-pushed the feat/docs-search-route branch from 750d388 to 05170b8 Compare November 4, 2026 14:38

@hansi-codes hansi-codes Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Tier B · See the inline comments. Summary

Comment thread src/mcp_server_appwrite/http_app.py Resolved
The route is unauthenticated, so an oversized body should not be buffered in full.
@ChiragAgg5k
ChiragAgg5k merged commit 67377bd into main Nov 4, 2026
6 checks passed
@ChiragAgg5k
ChiragAgg5k deleted the feat/docs-search-route branch November 4, 2026 15:33
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