Skip to content

feat: push delivery for MCP Events behind the events-push flag - #139

Merged
ChiragAgg5k merged 1 commit into
mainfrom
feat/events-push
Oct 13, 2026
Merged

ChiragAgg5k merged 1 commit into
mainfrom
feat/events-push

Conversation

@ChiragAgg5k

@ChiragAgg5k ChiragAgg5k commented Oct 12, 2026 •

Copy link
Copy Markdown
Member

Closes #137.

Adds push delivery to MCP Events: events/stream is 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 new MCP_EVENTS_PUSH flag; webhook delivery is unchanged.

How it works

  • Protocol layer. events/stream is 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. notifications/events/{active,event,heartbeat,error,terminated}, each carrying _meta["io.modelcontextprotocol/subscriptionId"].
  • One webhook per stream. Each stream creates one managed Appwrite webhook (MCP events push …) with a random sub_ 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.
  • Routing between replicas. The ingress publishes each projected event on 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 as no_listener; an unreachable bus is bus_error. Listeners ping the bus every 10 s, so a frozen connection is noticed and the stream reports truncated: true on recovery.
  • Lifecycle. Revoked access or a deleted webhook ends the stream with terminated; failed renewals are reported as error and 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.
  • Limits. 25 streams per principal and 1000 per replica (-32013). events/subscribe with delivery.mode: "push" answers -32014 with data: {"feature": "deliveryMode", "value": "push"}.
  • Advertising. events/list advertises ["webhook", "push"] only when the flag is on and a bus URL is set; otherwise events/stream is -32601.
  • Envelope. Push records use the new 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.
  • Telemetry. Deliveries carry a mode label; new mcp.events.streams up-down counter; stream outcomes opened, 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 to redis-server. The full suite also passed against Dragonfly in a Linux container. Compose now runs Dragonfly.

Follow-ups

  • Infra. The assets clusters have no Dragonfly yet (only an empty dragonfly-operator namespace; no operator or CRDs), so there is nothing to reuse. Push needs the operator in cluster-configuration and an instance plus MCP_EVENTS_REDIS_URL for mcp in application-configuration before the flag can be turned on.
  • Grafana panels for the new metrics, and a staging run with a real client.

Related: #127

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
@hansi-codes

hansi-codes Bot commented Oct 12, 2026

Copy link
Copy Markdown

🔵 Tier A · Mergeable after minor fixes

The near-expiry token window can keep a stream open later than intended, and enabled-mode push advertisement is not covered by an e2e assertion; both are minor issues.

This PR adds flag-gated MCP Events push delivery through long-lived events/stream requests, backed by managed Appwrite webhooks and Redis-protocol pub/sub between replicas. It also adds stream lifecycle handling, limits, telemetry, e2e coverage, and local/CI Dragonfly setup while preserving webhook delivery.

1 of 23 changed files was too large to include in full.

Verdict New comments Fixed Still open
💬 Commented 2 0 0

Note

Not approving while a bug finding is open: Close streams already inside the token-expiry grace window

Finding Where
🟡 Close streams already inside the token-expiry grace window src/mcp_server_appwrite/events/streams.py:448
🟡 Test push advertisement with the flag enabled src/mcp_server_appwrite/events/protocol.py:102
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
File Change
.github/workflows/ci.yml, compose.yaml Install Dragonfly in e2e CI and add a Dragonfly service for local compose.
AGENTS.md, docs/development.md, docs/events.md, docs/flags.md Document push delivery, its flag, lifecycle, routing, configuration, and testing.
pyproject.toml, uv.lock Add Redis support and update the subscription-envelope dependency.
src/mcp_server_appwrite/events/bus.py Add Redis pub/sub event routing and listener liveness checks.
src/mcp_server_appwrite/events/delivery.py, src/mcp_server_appwrite/events/envelope.py, src/mcp_server_appwrite/events/errors.py Share event occurrence encoding and support push-mode subscription records and errors.
src/mcp_server_appwrite/events/ingress.py, src/mcp_server_appwrite/events/projection.py Publish projected push events through the bus and track push-specific drop outcomes.
src/mcp_server_appwrite/events/protocol.py, src/mcp_server_appwrite/events/streams.py Register and serve events/stream, including notifications, renewal, limits, and shutdown.
src/mcp_server_appwrite/events/subscriptions.py, src/mcp_server_appwrite/events/webhooks.py Create, renew, clean up, and label push-backed Appwrite webhooks.
src/mcp_server_appwrite/flags.py Add the MCP_EVENTS_PUSH flag.
src/mcp_server_appwrite/http_app.py, src/mcp_server_appwrite/server.py Wire push streams into hosted HTTP and close them during Uvicorn shutdown.
src/mcp_server_appwrite/telemetry.py Add stream counts and delivery-mode telemetry.
tests/e2e/support.py, tests/e2e/test_events_stream.py Add real-bus e2e support and flows for push delivery and stream lifecycle.

Reviewed 1bef7fc · 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

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)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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.

Comment on lines +102 to +106
delivery = [Mode.WEBHOOK.value, *([Mode.PUSH.value] if push else [])]
return {
"name": event.name,
"description": event.description,
"delivery": [WEBHOOK],
"delivery": delivery,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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.

@ChiragAgg5k ChiragAgg5k self-assigned this Oct 13, 2026
@ChiragAgg5k
ChiragAgg5k merged commit 78a3dc8 into main Oct 13, 2026
6 checks passed
@ChiragAgg5k
ChiragAgg5k deleted the feat/events-push branch October 13, 2026 08:50
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.

feat(events): push delivery (events/stream)

1 participant