-
Notifications
You must be signed in to change notification settings - Fork 251
Add implementing-server-sent-events skill #265
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| ## Summary | ||
| Adds the **minimal-api-file-upload** skill for handling file uploads in ASP.NET Core 8 minimal APIs. | ||
|
|
||
| > **Note:** Replaces #134 (migrated from skills-old repo). Skill moved to `aspnetcore` plugin per repo restructuring. | ||
|
|
||
| ## What the Skill Teaches | ||
| - `IFormFile` binding in minimal APIs (when `[FromForm]` is needed vs automatic) | ||
| - `IFormFileCollection` for multiple file uploads | ||
| - Security: file size limits, content type allowlists, magic byte validation | ||
| - Safe file naming with `Guid` + validated extension (never trust client filename) | ||
| - Streaming uploads with `MultipartReader` for large files | ||
| - Antiforgery considerations | ||
| - Path traversal prevention | ||
|
|
||
| ## Eval Scenarios | ||
| - (eval.yaml needs scenarios — currently empty) | ||
|
|
||
| ## Files | ||
| - `plugins/aspnetcore/plugin.json` — ASP.NET Core plugin | ||
| - `plugins/aspnetcore/skills/minimal-api-file-upload/SKILL.md` — skill instructions | ||
| - `tests/aspnetcore/minimal-api-file-upload/eval.yaml` — eval scenarios (to be added) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| ## Summary | ||
| Adds the **implementing-server-sent-events** skill for building SSE endpoints in ASP.NET Core 8 minimal APIs. | ||
|
|
||
| > **Note:** Replaces #139 (migrated from skills-old repo). Skill moved to `aspnetcore` plugin per repo restructuring. | ||
|
|
||
| ## What the Skill Teaches | ||
| - Manual SSE protocol implementation (no built-in `MapSSE` — doesn't exist) | ||
| - Required headers: `Content-Type: text/event-stream`, `Cache-Control: no-cache`, `Connection: keep-alive` | ||
| - Correct SSE event framing with `\n\n` terminators | ||
| - `Response.Body.FlushAsync()` for immediate delivery | ||
| - Event IDs + `Last-Event-ID` header for reconnection support | ||
| - `retry:` field for controlling client reconnection interval | ||
| - `X-Accel-Buffering: no` for reverse proxy compatibility | ||
| - `HttpContext.RequestAborted` / `CancellationToken` for client disconnect detection | ||
| - `Channel<T>` for background service → endpoint communication | ||
|
|
||
| ## Eval Scenarios | ||
| 1. Implement SSE notification endpoint in ASP.NET Core 8 minimal API | ||
|
|
||
| ## Files | ||
| - `plugins/aspnetcore/plugin.json` — ASP.NET Core plugin | ||
| - `plugins/aspnetcore/skills/implementing-server-sent-events/SKILL.md` — skill instructions | ||
| - `tests/aspnetcore/implementing-server-sent-events/eval.yaml` — 1 eval scenario (needs expansion) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| ## Summary | ||
| Adds the **implementing-websocket-endpoints** skill for building WebSocket endpoints in ASP.NET Core 8. | ||
|
|
||
| > **Note:** Replaces #142 (migrated from skills-old repo). Skill moved to `aspnetcore` plugin per repo restructuring. | ||
|
|
||
| ## What the Skill Teaches | ||
| - `UseWebSockets()` middleware setup (no `MapWebSocket` — doesn't exist) | ||
| - Manual WebSocket upgrade via `AcceptWebSocketAsync` | ||
| - Proper receive loop with `EndOfMessage` fragment handling | ||
| - **Critical:** Use `CloseOutputAsync` (not `CloseAsync`) when responding to client-initiated close to avoid deadlock | ||
| - `ConcurrentDictionary`-based connection manager for broadcast | ||
| - `KeepAliveInterval` and `KeepAliveTimeout` configuration | ||
| - `AllowedOrigins` for cross-origin protection | ||
| - Query string token authentication (browser WebSocket API cannot send custom headers) | ||
|
|
||
| ## Eval Scenarios | ||
| 1. Implement WebSocket chat endpoint in ASP.NET Core 8 | ||
| 2. Fix WebSocket CloseAsync deadlock | ||
| 3. WebSocket should not be used for server-to-client streaming (negative test) | ||
| 4. Handle fragmented WebSocket messages correctly | ||
|
|
||
| ## Files | ||
| - `plugins/aspnetcore/plugin.json` — ASP.NET Core plugin | ||
| - `plugins/aspnetcore/skills/implementing-websocket-endpoints/SKILL.md` — skill instructions | ||
| - `tests/aspnetcore/implementing-websocket-endpoints/eval.yaml` — 4 eval scenarios |
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
| @@ -0,0 +1,31 @@ | ||||
| ## Summary | ||||
| Adds the **implementing-rate-limiting** skill for ASP.NET Core's built-in rate limiting middleware (.NET 7+). | ||||
|
|
||||
| > **Note:** Replaces #131 (migrated from skills-old repo). Skill moved to `aspnetcore` plugin per repo restructuring. | ||||
|
|
||||
| ## What the Skill Teaches | ||||
| - Algorithm selection: fixed window vs sliding window vs token bucket vs concurrency limiter | ||||
| - **Critical:** Setting `RejectionStatusCode = 429` (default is 503!) | ||||
| - Per-IP and per-user partitioned rate limiting | ||||
| - Middleware pipeline ordering (`UseRateLimiter()` after `UseRouting()`) | ||||
| - `OnRejected` callback with `Retry-After` header | ||||
| - `DisableRateLimiting()` for health check endpoints | ||||
| - Named policies with `RequireRateLimiting` | ||||
|
|
||||
| ## Eval Results (3-run, `claude-opus-4.6`) | ||||
|
|
||||
| | Scenario | Baseline | With Skill | Verdict | | ||||
| |----------|----------|------------|---------| | ||||
| | Add per-client rate limiting to an ASP.NET Core API | 4.0/5 | **4.7/5** | ❌ [1] | | ||||
| | Rate limiting silently inactive without UseRateLimiter | 4.3/5 | 2.7/5 | ❌ | | ||||
| | Fix rate limiter returning 503 instead of 429 | 2.3/5 | 2.3/5 | ❌ [2] | | ||||
| | Rate limiting should not apply to authentication scenarios | 4.0/5 | **4.7/5** | ✅ | | ||||
| | Choose correct rate limiting algorithm | 4.0/5 | **5.0/5** | ✅ | | ||||
|
|
||||
| [1] Quality improved but weighted score penalized by token/tool/time overhead. | ||||
| [2] Timeouts impacted scoring. Will iterate with increased timeouts. | ||||
|
|
||||
| ## Files | ||||
| - `plugins/aspnetcore/plugin.json` — new ASP.NET Core plugin | ||||
| - `plugins/aspnetcore/skills/implementing-rate-limiting/SKILL.md` — skill instructions | ||||
| - `tests/aspnetcore/implementing-rate-limiting/eval.yaml` — 5 eval scenarios | ||||
|
||||
| - `tests/aspnetcore/implementing-rate-limiting/eval.yaml` — 5 eval scenarios |
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,207 @@ | ||||||||||||||
| --- | ||||||||||||||
| name: implementing-server-sent-events | ||||||||||||||
| description: Implement Server-Sent Events (SSE) endpoints in ASP.NET Core. Use when building real-time streaming from server to client without WebSockets. | ||||||||||||||
| --- | ||||||||||||||
|
Comment on lines
+1
to
+4
|
||||||||||||||
|
|
||||||||||||||
| # Implementing Server-Sent Events (SSE) in ASP.NET Core | ||||||||||||||
|
|
||||||||||||||
| ## When to Use | ||||||||||||||
| - Server-to-client real-time push (notifications, live updates, streaming progress) | ||||||||||||||
| - When you DON'T need bidirectional communication (use WebSockets for that) | ||||||||||||||
| - SSE is simpler than WebSockets and works over standard HTTP | ||||||||||||||
| - Automatic reconnection built into EventSource browser API | ||||||||||||||
|
|
||||||||||||||
| ## When Not to Use | ||||||||||||||
| - Bidirectional communication needed → use WebSockets | ||||||||||||||
| - Binary data streaming → use WebSockets or gRPC streaming | ||||||||||||||
| - Need more than 6 concurrent connections per domain in HTTP/1.1 → use HTTP/2 | ||||||||||||||
|
|
||||||||||||||
| ## Inputs | ||||||||||||||
|
|
||||||||||||||
| | Input | Required | Description | | ||||||||||||||
| |-------|----------|-------------| | ||||||||||||||
| | Event data source | Yes | The data to stream (IAsyncEnumerable, Channel, timer, etc.) | | ||||||||||||||
| | Event types | No | Named event types for `event:` field | | ||||||||||||||
| | Client reconnection | No | Whether to support Last-Event-ID reconnection | | ||||||||||||||
|
|
||||||||||||||
|
Comment on lines
+21
to
+26
|
||||||||||||||
| ## Workflow | ||||||||||||||
|
|
||||||||||||||
| ### Step 1: CRITICAL — There Is No Built-In MapSSE() or MapServerSentEvents() | ||||||||||||||
|
|
||||||||||||||
| ASP.NET Core has NO built-in SSE endpoint helper. You must manually write the SSE protocol. | ||||||||||||||
|
|
||||||||||||||
| ```csharp | ||||||||||||||
| // COMMON MISTAKE: Trying to use a non-existent API | ||||||||||||||
| // app.MapSSE("/events", ...); // DOES NOT EXIST | ||||||||||||||
| // app.MapServerSentEvents("/events", ...); // DOES NOT EXIST | ||||||||||||||
| // app.UseServerSentEvents(); // DOES NOT EXIST | ||||||||||||||
|
|
||||||||||||||
| // CORRECT: Use a standard minimal API endpoint with manual SSE protocol | ||||||||||||||
| app.MapGet("/events", async (HttpContext context, CancellationToken ct) => | ||||||||||||||
| { | ||||||||||||||
| // CRITICAL: Set these three headers for SSE | ||||||||||||||
| context.Response.Headers.ContentType = "text/event-stream"; | ||||||||||||||
| context.Response.Headers.CacheControl = "no-cache"; | ||||||||||||||
|
Comment on lines
+43
to
+44
|
||||||||||||||
| context.Response.Headers.ContentType = "text/event-stream"; | |
| context.Response.Headers.CacheControl = "no-cache"; | |
| context.Response.ContentType = "text/event-stream"; | |
| context.Response.Headers["Cache-Control"] = "no-cache"; |
Copilot
AI
Mar 6, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The StreamWriter example is misleading: WriteLineAsync appends TextWriter.NewLine (often \r\n on Windows), not a single \n as the comment states. For SSE it’s safer to explicitly write \n\n (or set writer.NewLine = "\n") so event framing is consistent across OSes.
| writer.AutoFlush = true; // Flushes after every Write | |
| await writer.WriteLineAsync($"data: {msg}\n"); // Note: WriteLine adds one \n, we add one more | |
| writer.AutoFlush = true; // Flushes after every Write | |
| writer.NewLine = "\n"; // Ensure consistent '\n' line endings across OSes | |
| await writer.WriteLineAsync($"data: {msg}"); // Writes "data" line ending with '\n' | |
| await writer.WriteLineAsync(); // Writes the blank line that terminates the SSE event |
Copilot
AI
Mar 6, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This reconnection example assumes Last-Event-ID is an int (int.Parse(lastEventId)), but SSE event IDs are defined as opaque strings and the header can contain non-numeric values. Prefer treating it as a string (or using int.TryParse if you want to keep the numeric example) to avoid throwing and to better match the SSE spec.
| var startFrom = lastEventId != null ? int.Parse(lastEventId) + 1 : 0; | |
| var startFrom = 0; | |
| if (!string.IsNullOrEmpty(lastEventId) && int.TryParse(lastEventId, out var parsedId)) | |
| { | |
| startFrom = parsedId + 1; | |
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR description file claims
plugins/aspnetcore/plugin.jsonis part of the change and that the eval has "1 eval scenario (needs expansion)", but the PR as provided does not addplugins/aspnetcore/plugin.json, and the eval.yaml now contains multiple scenarios. Please update or remove this description file so it matches the actual changed files/structure.