feat: expose durable Global Ask through authenticated MCP - #655
Conversation
…ance' into feat/mcp-global-ask-current-contract
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
1a9f968
into
fix/global-ask-graph-fact-provenance
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None: | ||
| """Add Retry-After when a tool call marked the current request.""" | ||
|
|
||
| async def send_with_retry(message: Message) -> None: | ||
| """Attach the request-scoped quota delay to the response start.""" | ||
| retry_after = scope.get("state", {}).get(_RETRY_AFTER_STATE_KEY) | ||
| if message.get("type") == "http.response.start" and isinstance( | ||
| retry_after, int | ||
| ): | ||
| headers = [ | ||
| (name, value) | ||
| for name, value in message.get("headers", []) | ||
| if name.lower() != b"retry-after" | ||
| ] | ||
| headers.append((b"retry-after", str(retry_after).encode("ascii"))) | ||
| message = {**message, "headers": headers} | ||
| await send(message) | ||
|
|
||
| await self._app(scope, receive, send_with_retry) |
There was a problem hiding this comment.
🔍 Retry-After header relies on shared scope state
A rate-limit rejection stores retry_after_seconds on request.scope['state'] in _account (backend/app/mcp_server.py:157-161), and McpRetryAfterHeaderApp reads scope['state'] at response time to add the header. This works only if the SDK's request scope is the same ASGI scope dict the outer middleware sees. If the SDK builds a separate scope for JSON-RPC dispatch, the header is silently omitted; the structured retry_after_seconds data still returns.
Was this helpful? React with 👍 or 👎 to provide feedback.
| body = bytearray() | ||
| while True: | ||
| message = await receive() | ||
| message_type = message.get("type") | ||
| if message_type == "http.disconnect": | ||
| await _send_error(send, 400, "mcp_request_disconnected") | ||
| return | ||
| if message_type != "http.request": | ||
| await _send_error(send, 400, "mcp_invalid_request_body") | ||
| return | ||
| chunk = message.get("body", b"") | ||
| if not isinstance(chunk, bytes): | ||
| await _send_error(send, 400, "mcp_invalid_request_body") | ||
| return | ||
| if len(body) + len(chunk) > self._maximum_bytes: | ||
| await _send_error(send, 413, "mcp_request_too_large") | ||
| return | ||
| body.extend(chunk) | ||
| if not message.get("more_body", False): | ||
| break |
There was a problem hiding this comment.
📝 Info: Body admission does not bound empty chunk streams
The read loop caps total bytes but a client sending endless empty http.request chunks with more_body=True never exceeds the cap and never breaks (backend/app/mcp_admission.py:38-56). Each iteration awaits receive(), so it is not a busy loop, but the coroutine and connection stay open indefinitely. No chunk-count or time bound guards this.
Was this helpful? React with 👍 or 👎 to provide feedback.
| if not account.has_permission("post_read"): | ||
| raise HTTPException(status.HTTP_403_FORBIDDEN, "post_read permission required") | ||
| normalized_question = question.strip() | ||
| if not normalized_question: | ||
| raise HTTPException( | ||
| status.HTTP_422_UNPROCESSABLE_CONTENT, "question is required" | ||
| ) |
There was a problem hiding this comment.
📝 Info: Permission check now precedes blank-question check
The old REST ask_agent returned 422 for a blank question before enforcing post_read. The shared service checks permission first (backend/app/global_ask_service.py:29-35), so a provisioned account without post_read submitting a blank question now gets 403 instead of 422. The status for that edge case changed.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Outcome
Expose Global Ask to authenticated MCP clients without reviving the historical synchronous Ask fork. MCP submits and reads the same durable job contract as REST.
Contract
submit_global_askandread_global_ask_jobpreserve verification opt-in, knowledge cutoff, owner scope, citations, and limitationsADR 0218 and the PRD/gap baseline define the acceptance boundary. Historical #270/#286/#258 were used only as reusable implementation evidence.
Verification
uv run --extra dev --extra backend pytest -q: 1,435 passed, 17 skippeddocker compose config --quietgit diff --checkThe live MCP/k6 capacity acceptance remains explicitly open until an application-ready synthetic deployment supplies measured quota values. No production-capacity claim is made.
Stacked on #632; merge the parent first.