Repository navigation
feat: push delivery for MCP Events behind the events-push flag - #139
Conversation
Clients that hold a connection open no longer need a public webhook endpoint: events/stream delivers a subscription's events on the request itself. Each stream owns one leased Appwrite webhook, and events reach the replica holding the stream over Redis-protocol pub/sub (Dragonfly in the cluster). Closes #137
🔵 Tier A · Mergeable after minor fixes
This PR adds flag-gated MCP Events push delivery through long-lived 1 of 23 changed files was too large to include in full.
Note Not approving while a bug finding is open: Close streams already inside the token-expiry grace window
Fix with agent prompt### Issue 1
src/mcp_server_appwrite/events/streams.py:448
**Close streams already inside the token-expiry grace window**
When a token has less than `settings.expiry` seconds remaining, this branch waits until the token actually expires rather than ending immediately. The stream can therefore continue delivering events during that remaining window, contrary to the intended one-minute early close; sleeping for `max(ending - now, 0)` handles both near-expiry and already-expired tokens.
### Issue 2
src/mcp_server_appwrite/events/protocol.py:102-106
**Test push advertisement with the flag enabled**
The e2e flows verify webhook-only advertisement when push is off, but do not assert that enabling push adds it to `events/list`. A regression that leaves the catalog webhook-only would make push undiscoverable while the stream itself still works.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.📂 Walkthrough · 12
Reviewed |
| return | ||
| now = self._streams.now() / 1000 | ||
| ending = self._expires - self._streams.settings.expiry | ||
| await anyio.sleep(max(ending if ending > now else self._expires, now) - now) |
There was a problem hiding this comment.
Close streams already inside the token-expiry grace window
When a token has less than settings.expiry seconds remaining, this branch waits until the token actually expires rather than ending immediately. The stream can therefore continue delivering events during that remaining window, contrary to the intended one-minute early close; sleeping for max(ending - now, 0) handles both near-expiry and already-expired tokens.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/mcp_server_appwrite/events/streams.py
Line: 448
Comment:
**Close streams already inside the token-expiry grace window**
When a token has less than `settings.expiry` seconds remaining, this branch waits until the token actually expires rather than ending immediately. The stream can therefore continue delivering events during that remaining window, contrary to the intended one-minute early close; sleeping for `max(ending - now, 0)` handles both near-expiry and already-expired tokens.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.🟡 Minor · bug · Reply if this doesn't apply.
| delivery = [Mode.WEBHOOK.value, *([Mode.PUSH.value] if push else [])] | ||
| return { | ||
| "name": event.name, | ||
| "description": event.description, | ||
| "delivery": [WEBHOOK], | ||
| "delivery": delivery, |
There was a problem hiding this comment.
Test push advertisement with the flag enabled
The e2e flows verify webhook-only advertisement when push is off, but do not assert that enabling push adds it to events/list. A regression that leaves the catalog webhook-only would make push undiscoverable while the stream itself still works.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/mcp_server_appwrite/events/protocol.py
Line: 102-106
Comment:
**Test push advertisement with the flag enabled**
The e2e flows verify webhook-only advertisement when push is off, but do not assert that enabling push adds it to `events/list`. A regression that leaves the catalog webhook-only would make push undiscoverable while the stream itself still works.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.🟡 Minor · testing · Reply if this doesn't apply.
Closes #137.
Adds push delivery to MCP Events:
events/streamis a long-lived request that streams a subscription's events back on the request itself, so clients that hold a connection open no longer need a public webhook endpoint. Behind the newMCP_EVENTS_PUSHflag; webhook delivery is unchanged.How it works
events/streamis an ordinary SDK request handler. On mcp 2.3.0's stateless SSE transport a handler may run indefinitely, its notifications are written as SSE frames on that POST, and a client abort cancels it, on both the 2026-07-28 and 2025-11-25 protocols. No separate ASGI route was needed.notifications/events/{active,event,heartbeat,error,terminated}, each carrying_meta["io.modelcontextprotocol/subscriptionId"].MCP events push …) with a randomsub_id and a 5-minute lease, renewed every 2 minutes with the stream's own token and deleted on close. If a replica dies, the lease expires and cleanup removes the webhook a minute later.mcp:events:<id>over Redis-protocol pub/sub (MCP_EVENTS_REDIS_URL; Dragonfly in the cluster), and the replica holding the stream forwards it. Events published while nobody listens are acknowledged and counted asno_listener; an unreachable bus isbus_error. Listeners ping the bus every 10 s, so a frozen connection is noticed and the stream reportstruncated: trueon recovery.terminated; failed renewals are reported aserrorand retried; a stream ends cleanly a minute before its token expires so clients reconnect with a fresh one. SIGTERM closes every stream, deletes its webhook and sends the final result.-32013).events/subscribewithdelivery.mode: "push"answers-32014withdata: {"feature": "deliveryMode", "value": "push"}.events/listadvertises["webhook", "push"]only when the flag is on and a bus URL is set; otherwiseevents/streamis-32601.delivery: {"mode": "push"}from mcp-event-subscriptions 0.1.0a2 (feat: push delivery mode for subscription records ChiragAgg5k/mcp-event-subscriptions#1). Webhook records are byte-compatible.modelabel; newmcp.events.streamsup-down counter; stream outcomesopened,closed,terminated,expired.Testing
14 new e2e flows in
tests/e2e/test_events_stream.py, two server instances sharing one bus: delivery between replicas on both protocol eras, renewal, abort, refusals and limits, revoked access, deleted webhook, failed and hung renewals, bus outage and frozen bus, token expiry, abort during create, shutdown, crash with lease expiry, and the flag off. CI runs them against Dragonfly v2.0.0 (the version the clusters run, checksum-pinned); locally they fall back toredis-server. The full suite also passed against Dragonfly in a Linux container. Compose now runs Dragonfly.Follow-ups
dragonfly-operatornamespace; no operator or CRDs), so there is nothing to reuse. Push needs the operator in cluster-configuration and an instance plusMCP_EVENTS_REDIS_URLformcpin application-configuration before the flag can be turned on.Related: #127