fix(evi): frame mcp post responses as sse when the client accepts it - #546
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
4 Skipped Deployments
|
|
|
Thank you for following the naming conventions! 🙏 |
📝 WalkthroughWalkthroughThe MCP channel now returns SSE-framed ChangesMCP response negotiation
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. 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 |
commit: |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/evi/agent/channels/mcp.ts`:
- Around line 71-73: Update the comment above the streamable HTTP response
handling to remove the specific `Linear` client reference and implementation
history. Keep only the durable protocol constraint: return one SSE message when
the request accepts `text/event-stream`.
- Line 74: Update the SSE selection condition in the MCP request handler to
parse the Accept header’s media ranges rather than using
includes('text/event-stream'). Match media types case-insensitively, allow
media-type parameters, and select SSE only when text/event-stream has effective
q greater than zero, including rejecting q=0. Add tests covering q=0, positive q
values, parameters, and case variants.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 1108dfa6-8d07-4394-95b8-2425f2f95995
📒 Files selected for processing (1)
apps/evi/agent/channels/mcp.ts
| // Streamable HTTP lets the server answer POSTs as JSON or as an SSE | ||
| // frame; clients built against SSE-framing servers (Linear's among | ||
| // them) expect the frame when their Accept says so. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Keep the comment focused on the durable protocol constraint.
The reference to Linear records implementation motivation and a specific client. Replace it with a vendor-neutral statement such as // Return one SSE message when the request accepts text/event-stream.
As per coding guidelines, comments should state only durable constraints and should not narrate implementation history.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/evi/agent/channels/mcp.ts` around lines 71 - 73, Update the comment
above the streamable HTTP response handling to remove the specific `Linear`
client reference and implementation history. Keep only the durable protocol
constraint: return one SSE message when the request accepts `text/event-stream`.
Source: Coding guidelines
| // Streamable HTTP lets the server answer POSTs as JSON or as an SSE | ||
| // frame; clients built against SSE-framing servers (Linear's among | ||
| // them) expect the frame when their Accept says so. | ||
| if (req.headers.get('accept')?.includes('text/event-stream')) { |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
# Expect: negotiation code or tests cover q-values and case-insensitive media types.
rg -n -C 5 'Accept|text/event-stream|q=0|Text/Event-Stream' apps/eviRepository: HugoRCD/evlog
Length of output: 2006
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Target file outline and relevant section:\n'
ast-grep outline apps/evi/agent/channels/mcp.ts || true
sed -n '1,120p' apps/evi/agent/channels/mcp.ts
printf '\nPackage lock/typescript availability:\n'
(git ls-files | rg '(^|/)package\.json$|(^|/)pnpm-lock.yaml$|(^|/)package-lock\.json$|(^|/)yarn\.lock$' | sed -n '1,80p')
(node -e 'console.log("node", process.version)' 2>/dev/null || true)Repository: HugoRCD/evlog
Length of output: 4900
🌐 Web query:
RFC 9110 Accept header q=0 media type unacceptable case-insensitive media type type subtype
💡 Result:
In RFC 9110, the Accept header allows a user agent to specify media types that are acceptable for the response [1]. When a media type in the Accept header is assigned a quality value (q-value) of 0 (e.g., media-type;q=0), it indicates that the media type is "not acceptable" to the client [2]. Regarding media type casing, the type and subtype components of a media type are case-insensitive [3][4]. While they are traditionally written in lowercase, RFC 9110 and related standards treat them as case-insensitive identifiers [3][4]. Parameter names within media types are also case-insensitive, though the case-sensitivity of parameter values depends on the specific semantics of the parameter name [3][5].
Citations:
- 1: https://runebook.dev/en/docs/http/rfc9110/section-12.5.1
- 2: https://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html
- 3: https://www.rfc-editor.org/rfc/rfc9110.html
- 4: https://developer.mozilla.org/en-US/docs/Web/HTTP/Guides/MIME_types
- 5: https://www.rfc-editor.org/info/rfc9110/
🏁 Script executed:
#!/bin/bash
set -euo pipefail
node - <<'JS'
const examples = [
'text/event-stream;q=0, application/json;q=0.9',
'application/json;q=0.5, text/event-stream;q=0.1',
'TEXT/Event-Stream, application/json;q=0.5',
'TEXT/Event-STREAM; charset=utf-8, application/json',
'text/event-stream, application/json;q=0'
]
for (const accept of examples) {
const include = accept.includes('text/event-stream')
console.log(JSON.stringify({ accept, includes_lowercase_text_event_stream: include }))
}
JS
printf '\nExisting test directories and mcp-related files:\n'
git ls-files | rg '(^|/)(test|specs|vitest|jest|node_modules|dist|build)' | sed -n '1,200p'
rg -n "mcp|Accept|event-stream|q=" apps/evi/agent apps packages -g '!node_modules' | sed -n '1,220p'Repository: HugoRCD/evlog
Length of output: 24464
Parse Accept media ranges before selecting SSE.
Use strict Accept negotiation instead of includes('text/event-stream') so SSE is returned only when text/event-stream has effective quality greater than zero. This rejects Accept: text/event-stream;q=0 correctly and matches case-insensitive media types such as TEXT/Event-Stream.
Add tests for q=0, positive q values, media-type parameters, and case variants.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/evi/agent/channels/mcp.ts` at line 74, Update the SSE selection
condition in the MCP request handler to parse the Accept header’s media ranges
rather than using includes('text/event-stream'). Match media types
case-insensitively, allow media-type parameters, and select SSE only when
text/event-stream has effective q greater than zero, including rejecting q=0.
Add tests covering q=0, positive q values, parameters, and case variants.
Raycast connects but the session setup fails against plain-JSON POST responses; Linear's MCP (which Raycast handles) frames POST responses as a single SSE message event. The channel now mirrors that: when the request's Accept includes text/event-stream, the JSON-RPC response is returned as an SSE frame with content-type text/event-stream; plain JSON otherwise. GET stays a spec-compliant 405. No changeset: confined to apps/evi. Verified: tsc, 63 unit tests, eve build; curl with both Accept variants validates on the preview.
Summary by CodeRabbit
text/event-streamnow receive responses in Server-Sent Events format.