Repository navigation
Conversation
Signed-off-by: Djordje Lukic <djordje.lukic@docker.com>
rumpl
force-pushed
the
feat/experimental-mcp-events
branch
from
November 10, 2026 12:44
46d5495 to
d0d117f
Compare
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Custom event RPC failures bypass session error sanitization, allowing endpoint query credentials to leak into model-facing errors or stream logs.
8 open issues
What changed in this PR
Adds opt-in experimental MCP Events support, connecting push subscriptions over stdio and Streamable HTTP to the agent’s existing recall/steering system.
Changes:
- Adds event discovery, subscription, cancellation, heartbeat monitoring, cursor tracking, and bounded delivery.
- Introduces per-toolset configuration and shared-definition inheritance, with schemas and generated TypeScript types.
- Adds demo servers, an HTTP agent configuration, documentation, and integration tests.
| File | Description |
|---|---|
sdk/typescript/src/generated/config.ts |
Adds experimental Events configuration types. |
sdk/typescript/schemas/docker-agent.schema.json |
Updates generated configuration schemas. |
pkg/tools/mcp/stdio.go |
Enables stdio event capability and transport adapter. |
pkg/tools/mcp/session_remote.go |
Integrates events into session-owned HTTP clients. |
pkg/tools/mcp/session_client.go |
Stores event state and closes subscriptions. |
pkg/tools/mcp/remote.go |
Enables events for regular Streamable HTTP clients. |
pkg/tools/mcp/mcp.go |
Applies configuration and exposes event tools. |
pkg/tools/mcp/events.go |
Implements discovery and subscription lifecycle management. |
pkg/tools/mcp/events_test.go |
Tests stdio delivery, correlation, and cancellation. |
pkg/tools/mcp/events_http.go |
Adapts HTTP requests and filters event notifications. |
pkg/tools/mcp/events_http_test.go |
Tests concurrent HTTP subscriptions and stream filtering. |
pkg/tools/mcp/events_experimental_test.go |
Tests opt-in configuration and disabled defaults. |
pkg/config/mcps.go |
Inherits experimental settings from shared definitions. |
pkg/config/mcps_test.go |
Tests inheritance and explicit overrides. |
pkg/config/latest/validate.go |
Restricts experimental settings to MCP toolsets. |
pkg/config/latest/validate_test.go |
Tests the toolset restriction. |
pkg/config/latest/types.go |
Defines experimental Events configuration. |
examples/mcp-events/README.md |
Documents demos, configuration, and limitations. |
examples/mcp-events/main.go |
Adds a wire-level stdio event server. |
examples/mcp-events/http.go |
Adds HTTP serving and lifecycle logging. |
examples/mcp-events/http_test.go |
Tests HTTP delivery and logging behavior. |
examples/mcp-events/agent-http.yaml |
Configures an opted-in local HTTP demo agent. |
docs/configuration/tools/index.md |
Documents experimental Events support. |
agent-schema.json |
Adds the experimental toolset configuration property. |
Review effort: Balanced
Comment on lines
+308
to
+310
| result, err := gomcp.CallCustomMethod[*eventParams, *eventListResult](ctx, s, "events/list", &eventParams{Cursor: cursor}) | ||
| if err != nil { | ||
| return nil, err |
Comment on lines
+2551
to
+2555
| "experimental": { | ||
| "type": "object", | ||
| "description": "Experimental MCP features. Disabled by default.", | ||
| "additionalProperties": false, | ||
| "properties": { |
Comment on lines
+355
to
+356
| case <-lifetime.Done(): | ||
| return "", errors.New("MCP event subscription stopped before confirmation") |
Comment on lines
+399
to
+402
| if err != nil && ctx.Err() == nil { | ||
| slog.WarnContext(ctx, "MCP event stream ended", "event", p.Name, "error", err) | ||
| } | ||
| return |
Comment on lines
+429
to
+434
| p.Cursor = n.Cursor | ||
| if n.Truncated && n.Cursor != nil { | ||
| if err := rt.Recall(ctx, fmt.Sprintf("MCP event stream %q from %q skipped events; fetch authoritative state with tools if needed.", p.Name, source)); err != nil { | ||
| slog.WarnContext(ctx, "MCP event gap delivery failed", "error", err) | ||
| } | ||
| } |
Comment on lines
+25
to
+27
| if req.Method != http.MethodPost || req.GetBody == nil { | ||
| return t.base.RoundTrip(req) | ||
| } |
Comment on lines
+59
to
+63
| clone := req.Clone(req.Context()) | ||
| clone.Body = io.NopCloser(bytes.NewReader(data)) | ||
| clone.ContentLength = int64(len(data)) | ||
| clone.GetBody = func() (io.ReadCloser, error) { return io.NopCloser(bytes.NewReader(data)), nil } | ||
| resp, err := t.base.RoundTrip(clone) |
| if err != nil { | ||
| return nil, err | ||
| } | ||
| if mcpToolset, ok := ts.(*Toolset); ok { |
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Event-discovery and stream errors bypass credential sanitization, potentially exposing endpoint query credentials to the model and logs.
10 open issues
Review effort: Balanced
| if len(s.streams) >= 16 { | ||
| s.mu.Unlock() | ||
| cancel() | ||
| return s.failure(req.ID, -32023, "Subscription limit reached") |
| Run from the repository root: | ||
|
|
||
| ```sh | ||
| ./bin/docker-agent run examples/mcp-events/agent.yaml |
aheritier
added
area/config
aheritier
requested changes
Nov 11, 2026
aheritier
left a comment
aheritier
left a comment
Collaborator
There was a problem hiding this comment.
See inline comments. Please address the blocking feedback and mark the PR ready for re-review.
Prior threads:
- 4525070953: Still open — [blocking, high confidence] custom event RPC errors bypass the owning session's sanitizer.
- 4525071010: Still open — [should-fix, high confidence] the shared MCP definition schema rejects the documented experimental setting.
- 4525071058: Still open — [blocking, high confidence] completion can race with queued subscription readiness and report a false failure.
- 4525071122: Still open — [blocking, high confidence] a recoverable HTTP stream interruption permanently removes the subscription.
- 4525071175: Still open — [blocking, high confidence] rejected gap delivery leaves the cursor advanced without notifying the agent.
- 4525071225: Still open — [blocking, high confidence] resumed HTTP GET responses bypass event interception and recall delivery.
- 4525071280: Still open — [blocking, high confidence] events/stream omits the proposal's required Mcp-Name header.
- 4525071324: Still open — [blocking, high confidence] gateway-backed docker references silently ignore the experimental opt-in.
- 4525133185: Still open — [should-fix, high confidence] the demo reports subscription exhaustion with the NotFound error code.
- 4525133236: Still open — [should-fix, high confidence] the documented stdio agent configuration is absent.
| slog.WarnContext(ctx, "MCP event stream ended", "event", p.Name, "error", err) | ||
| } | ||
| return | ||
| case <-timer.C: |
Collaborator
There was a problem hiding this comment.
[blocking] [high confidence] Add regression tests that exercise the subscription state machine, not just successful delivery and adapter cleanup. The added tests do not drive this confirmation-timeout/reopen branch, cursor replay and deduplication, recoverable stream failure versus normal completion, termination, or rejected Recall delivery. TestEventCorrelation only checks notification routing and cancellation, and the HTTP integration sends one successful stream; neither verifies these recovery transitions. Introduce a controllable timeout/clock and scripted stream outcomes, then assert readiness, cancellation, retry cursors, duplicate suppression, and delivery-failure behavior. These are new behavior-bearing paths, and the unresolved delivery/recovery threads demonstrate failures the existing tests do not catch.
aheritier
marked this pull request as draft
November 11, 2026 23:33
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
experimental.mcp_events: true(disabled by default), including shared MCP definition inheritance.Scope
Targets proposal revision
b08e4f9dd684f39d846d94089487fbeb28e666f5. Polling, webhooks, legacy SSE event delivery, durable cursors, and subscription restoration after connection replacement are not implemented.Try it
go run ./examples/mcp-events -listen 127.0.0.1:8080 # In another terminal: ./bin/docker-agent run examples/mcp-events/agent-http.yamlAsk: "Subscribe to demo.message with interval_seconds 5 and count 3."