-
Notifications
You must be signed in to change notification settings - Fork 3
Jyh/sse endpoint #53
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
base: main
Are you sure you want to change the base?
Jyh/sse endpoint #53
Conversation
WalkthroughThe changes introduce Server-Sent Events (SSE) support alongside existing WebSocket functionality. New classes and modules implement SSE client and server endpoints, and the demo server is updated to use the SSE connector. The connector package's API is expanded to include SSE exports, and the client demo is modified to initialize and use the SSE endpoint for communication. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as SSEClientEndpoint
participant Server as SSEEndpointServer
participant Center as ConnectorCenter
participant Endpoint as SSEServerEndpoint
Client->>Server: Connect to /client (SSE)
Server->>Center: Register new SSEServerEndpoint
Server->>Client: Send INITIALIZE with clientId
Client->>Server: POST /message with JSON message
Server->>Endpoint: Receive message, invoke onmessage
Endpoint->>Client: Send SSE data message
Client->>Client: onmessage callback invoked
Poem
✨ Finishing Touches
🧪 Generate Unit Tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 7
🧹 Nitpick comments (6)
demo-server/src/index.ts (2)
5-5
: Remove unused import.The
createConnector
import is no longer used since the code has switched to SSE connector. This is also flagged by ESLint.-import { createConnector, createSSEConnector } from './connector'; +import { createSSEConnector } from './connector';
48-49
: Consider removing commented code.Instead of leaving the old WebSocket route commented out, consider removing it entirely if the transition to SSE is complete and the WebSocket functionality is no longer needed.
-// // connector -// (app as unknown as expressWs.WithWebsocketMethod).ws('/ws', websocketConnectionHandler);demo-server/src/connector.ts (1)
1-7
: Fix import issues flagged by static analysis.ESLint indicates type import inconsistencies and import sorting issues.
-import { - WebSocketServerEndpoint, - ConnectorCenter, - WebSocketEndpointServer, - SSEEndpointServer, - SSEServerEndpoint, -} from '@opentiny/tiny-agent-mcp-connector'; +import { + ConnectorCenter, + SSEEndpointServer, + WebSocketEndpointServer, + type SSEServerEndpoint, + type WebSocketServerEndpoint, +} from '@opentiny/tiny-agent-mcp-connector';packages/mcp/mcp-connector/src/sse/sse-endpoint-server.ts (1)
1-5
: Fix type import usage to align with ESLint configuration.The static analysis correctly identifies that some imports are only used as types.
-import * as http from 'node:http'; -import { ConnectorCenter } from '../connector-center'; -import { EndpointMessageType } from '../endpoint.type'; +import * as http from 'node:http'; +import type { ConnectorCenter } from '../connector-center'; +import { EndpointMessageType } from '../endpoint.type';packages/mcp/mcp-connector/src/sse/sse-client-endpoint.ts (1)
1-2
: Fix type import usage to align with ESLint configuration.The static analysis correctly identifies that some imports are only used as types.
-import { JSONRPCMessage } from '@modelcontextprotocol/sdk/types'; -import { EndpointMessageType, IConnectorEndpoint, IEndpointMessage } from '../endpoint.type'; +import type { JSONRPCMessage } from '@modelcontextprotocol/sdk/types'; +import { EndpointMessageType, type IConnectorEndpoint, type IEndpointMessage } from '../endpoint.type';packages/mcp/mcp-connector/src/sse/sse-server-endpoint.ts (1)
1-3
: Fix type import usage to align with ESLint configuration.The static analysis correctly identifies that some imports are only used as types.
-import * as http from 'node:http'; -import { JSONRPCMessage } from '@modelcontextprotocol/sdk/types'; -import { EndpointMessageType, IConnectorEndpoint, IEndpointMessage } from '../endpoint.type'; +import type * as http from 'node:http'; +import type { JSONRPCMessage } from '@modelcontextprotocol/sdk/types'; +import { EndpointMessageType, type IConnectorEndpoint, type IEndpointMessage } from '../endpoint.type';
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
demo-server/src/connector.ts
(2 hunks)demo-server/src/index.ts
(2 hunks)demo/src/mcp/index.js
(2 hunks)packages/mcp/mcp-connector/src/index.ts
(1 hunks)packages/mcp/mcp-connector/src/sse/index.ts
(1 hunks)packages/mcp/mcp-connector/src/sse/sse-client-endpoint.ts
(1 hunks)packages/mcp/mcp-connector/src/sse/sse-endpoint-server.ts
(1 hunks)packages/mcp/mcp-connector/src/sse/sse-server-endpoint.ts
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (5)
packages/mcp/mcp-connector/src/sse/sse-endpoint-server.ts (2)
packages/mcp/mcp-connector/src/connector-center.ts (1)
ConnectorCenter
(4-22)packages/mcp/mcp-connector/src/sse/sse-server-endpoint.ts (1)
SSEServerEndpoint
(5-57)
packages/mcp/mcp-connector/src/sse/sse-client-endpoint.ts (1)
packages/mcp/mcp-connector/src/endpoint.type.ts (2)
IConnectorEndpoint
(13-25)IEndpointMessage
(7-11)
demo-server/src/connector.ts (3)
packages/mcp/mcp-connector/src/connector-center.ts (1)
ConnectorCenter
(4-22)packages/mcp/mcp-connector/src/sse/sse-server-endpoint.ts (1)
SSEServerEndpoint
(5-57)packages/mcp/mcp-connector/src/sse/sse-endpoint-server.ts (1)
SSEEndpointServer
(7-45)
demo-server/src/index.ts (2)
demo-server/src/connector.ts (1)
createSSEConnector
(26-35)demo-server/src/proxy-server.ts (1)
createProxyServer
(105-124)
packages/mcp/mcp-connector/src/sse/sse-server-endpoint.ts (2)
packages/mcp/mcp-connector/src/endpoint.type.ts (2)
IConnectorEndpoint
(13-25)IEndpointMessage
(7-11)packages/mcp/mcp-connector/src/endpoint-transport.ts (1)
clientId
(6-8)
🪛 ESLint
packages/mcp/mcp-connector/src/sse/sse-endpoint-server.ts
[error] 2-2: All imports in the declaration are only used as types. Use import type
.
(@typescript-eslint/consistent-type-imports)
packages/mcp/mcp-connector/src/sse/sse-client-endpoint.ts
[error] 1-1: All imports in the declaration are only used as types. Use import type
.
(@typescript-eslint/consistent-type-imports)
[error] 2-2: Imports "IConnectorEndpoint" and "IEndpointMessage" are only used as type.
(@typescript-eslint/consistent-type-imports)
[error] 8-8: The 'EventSource' is still an experimental feature The configured version range is '^20.19.0 || >=22.12.0'.
(n/no-unsupported-features/node-builtins)
[error] 21-21: The 'EventSource' is still an experimental feature The configured version range is '^20.19.0 || >=22.12.0'.
(n/no-unsupported-features/node-builtins)
demo-server/src/connector.ts
[error] 1-7: Imports "WebSocketServerEndpoint" and "SSEServerEndpoint" are only used as type.
(@typescript-eslint/consistent-type-imports)
[error] 3-3: Member 'ConnectorCenter' of the import declaration should be sorted alphabetically.
(sort-imports)
demo-server/src/index.ts
[error] 5-5: 'createConnector' is defined but never used. Allowed unused vars must match /^_/u.
(@typescript-eslint/no-unused-vars)
packages/mcp/mcp-connector/src/sse/sse-server-endpoint.ts
[error] 1-1: All imports in the declaration are only used as types. Use import type
.
(@typescript-eslint/consistent-type-imports)
[error] 2-2: All imports in the declaration are only used as types. Use import type
.
(@typescript-eslint/consistent-type-imports)
[error] 3-3: Imports "IConnectorEndpoint" and "IEndpointMessage" are only used as type.
(@typescript-eslint/consistent-type-imports)
🔇 Additional comments (7)
packages/mcp/mcp-connector/src/index.ts (1)
5-5
: LGTM! Clean API extension.The export statement correctly extends the package's public API to include the SSE functionality, following the established pattern of other module exports.
packages/mcp/mcp-connector/src/sse/index.ts (1)
1-3
: LGTM! Standard module aggregation pattern.The index file correctly aggregates and re-exports the SSE-related classes, providing a clean single entry point for the SSE functionality.
demo-server/src/index.ts (1)
39-40
: Clean transition to SSE connector.The switch from WebSocket to SSE connector is implemented correctly. The SSE connector setup aligns with the new functionality.
demo/src/mcp/index.js (2)
12-13
: Clean SSE endpoint transport initialization.The switch to SSE endpoint transport is implemented correctly, with the WebSocket alternative properly commented as a fallback option.
34-42
: Good addition of SSE endpoint to return object.Including the
sseEndpoint
in the returned object provides access to the SSE endpoint instance for external management if needed.demo-server/src/connector.ts (1)
26-35
: Well-structured SSE connector implementation.The
createSSEConnector
function follows the same pattern as the existing WebSocket connector, providing consistent API design and proper resource initialization.packages/mcp/mcp-connector/src/sse/sse-client-endpoint.ts (1)
8-8
: ```shell
#!/bin/bashCorrectly search for an EventSource polyfill in dependencies and lockfiles
Check package.json for any dependency named "eventsource"
grep -R -i '"eventsource"' package.json || echo "No match in package.json"
Check package-lock.json and yarn.lock for installed polyfills or packages referencing eventsource
for lockfile in package-lock.json yarn.lock; do
if [ -f "$lockfile" ]; then
grep -R -i eventsource "$lockfile" && found=true
fi
doneif [ -z "$found" ]; then
echo "No references to 'eventsource' found in lockfiles."
fi</details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
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.
Actionable comments posted: 5
🧹 Nitpick comments (1)
docs/src/extensions/connector.md (1)
109-112
: Improve server startup error handling.
this.app.listen(config.port)
is called without a callback to report readiness or catch errors. Add a listener to log success or failure.@@ -109,3 +109,6 @@ class SSEEndpointServer { - this.app.listen(config.port); + this.app.listen(config.port, (err?: Error) => { + if (err) console.error('Failed to start SSE server:', err); + else console.log(`SSE server listening on port ${config.port}`); + });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
docs/.vitepress/config.mts
(1 hunks)docs/src/extensions/connector.md
(1 hunks)docs/src/extensions/extension.md
(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- docs/src/extensions/extension.md
- docs/.vitepress/config.mts
🔇 Additional comments (2)
docs/src/extensions/connector.md (2)
1-9
: No comments needed for introduction and section headings.
The high-level description and Chinese headings are clear and appropriately scoped.
10-24
: Interface definition is clear.
TheIConnectorEndpoint
snippet is correctly defined, matching TypeScript conventions and the intended protocol contract.
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.
Actionable comments posted: 1
♻️ Duplicate comments (6)
packages/mcp/mcp-connector/src/sse/sse-endpoint-server.ts (1)
25-25
: Add await and error handling for endpoint initialization.The endpoint.start() call is asynchronous but not awaited, which could lead to race conditions. This was flagged in previous reviews.
- endpoint.start(); + try { + await endpoint.start(); + } catch (error) { + console.error('Failed to start SSE endpoint:', error); + res.statusCode = 500; + res.end('Internal Server Error'); + return; + }demo-server/src/connector.ts (1)
26-37
: Fix duplicate start() call and make port configurable.The SSE server is started twice (lines 30 and 33), and the port is hardcoded. This creates confusion and limits flexibility.
-export function createSSEConnector() { +export function createSSEConnector(config?: { port?: number }) { const connectorCenter = new ConnectorCenter<SSEServerEndpoint>(); - const port = 8082; + const port = config?.port || process.env.SSE_PORT ? parseInt(process.env.SSE_PORT) : 8082; const sseEndpointServer = new SSEEndpointServer({ port }, connectorCenter); - sseEndpointServer.start(); try { sseEndpointServer.start(); } catch (error) { console.error(`Failed to start SSE endpoint server on port ${port}:`, error); throw error; } return { connectorCenter, sseEndpointServer, }; }packages/mcp/mcp-connector/src/sse/sse-client-endpoint.ts (1)
52-60
: Replace deprecated URL.parse and improve error handling.The implementation still uses deprecated
URL.parse
and lacks proper fetch response handling, as flagged in previous reviews.- const url = URL.parse(this.url); - - fetch(`${url?.origin}/message`, { - method: 'POST', - body: JSON.stringify(message), - headers: { - 'Content-Type': 'application/json', - }, - }); + const url = new URL(this.url); + + const response = await fetch(`${url.origin}/message`, { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + }, + body: JSON.stringify(message), + }); + + if (!response.ok) { + throw new Error(`HTTP error! status: ${response.status}`); + }packages/mcp/mcp-connector/src/sse/sse-server-endpoint.ts (1)
18-44
: Fix Promise resolution and prevent memory leaks.The start() method returns a Promise that never resolves, and adds event listeners without cleanup, as flagged in previous reviews.
async start(): Promise<void> { + return new Promise((resolve) => { + const requestHandler = (req: http.IncomingMessage, res: http.ServerResponse) => { - this.app.on('request', (req, res) => { if (req.url === '/message') { res.setHeader('Access-Control-Allow-Origin', '*'); res.setHeader('Access-Control-Allow-Methods', '*'); let body = ''; req.on('data', (chunk) => { body += chunk.toString(); }); req.on('end', () => { try { const message = JSON.parse(body); if (message.type === EndpointMessageType.INITIALIZE) { res.end(); return; } this.onmessage?.(message); } finally { res.end(); } }); } + }; + + this.app.on('request', requestHandler); + resolve(); - }); + }); }docs/src/extensions/connector.md (2)
46-76
: Fix Promise resolution in server example.The start() method in the documentation example returns a Promise that never resolves, which would cause indefinite hangs as flagged in previous reviews.
async start(): Promise<void> { + return new Promise((resolve) => { // 订阅http请求 this.app.on('request', (req, res) => { // 定义/message api 用以接收客户端的请求内容 if (req.url === '/message') { // ... existing code ... } }); + resolve(); + }); }
211-220
: Replace deprecated URL.parse in client example.The client example uses deprecated URL.parse as flagged in previous reviews.
- const url = URL.parse(this.url); - - // 请求/message api 向服务端发送请求 - fetch(`${url?.origin}/message`, { + const url = new URL(this.url); + + // 请求/message api 向服务端发送请求 + const response = await fetch(`${url.origin}/message`, { method: 'POST', body: JSON.stringify(message), headers: { 'Content-Type': 'application/json', }, }); + + if (!response.ok) { + throw new Error(`HTTP error! status: ${response.status}`); + }
🧹 Nitpick comments (4)
packages/mcp/mcp-connector/src/sse/sse-endpoint-server.ts (1)
2-2
: Fix import type consistency.The
ConnectorCenter
import is only used as a type and should be imported withimport type
for consistency.-import { ConnectorCenter } from '../connector-center'; +import type { ConnectorCenter } from '../connector-center';demo-server/src/connector.ts (1)
1-7
: Fix import type consistency and sorting.Multiple imports are only used as types and should use
import type
. Also, imports should be sorted alphabetically.-import { - WebSocketServerEndpoint, - ConnectorCenter, - WebSocketEndpointServer, - SSEEndpointServer, - SSEServerEndpoint, -} from '@opentiny/tiny-agent-mcp-connector'; +import { + WebSocketEndpointServer, + SSEEndpointServer, +} from '@opentiny/tiny-agent-mcp-connector'; +import type { + ConnectorCenter, + SSEServerEndpoint, + WebSocketServerEndpoint, +} from '@opentiny/tiny-agent-mcp-connector';packages/mcp/mcp-connector/src/sse/sse-client-endpoint.ts (1)
1-2
: Fix import type consistency.Several imports are only used as types and should use
import type
for consistency.-import { JSONRPCMessage } from '@modelcontextprotocol/sdk/types'; -import { EndpointMessageType, IConnectorEndpoint, IEndpointMessage } from '../endpoint.type'; +import type { JSONRPCMessage } from '@modelcontextprotocol/sdk/types'; +import { EndpointMessageType } from '../endpoint.type'; +import type { IConnectorEndpoint, IEndpointMessage } from '../endpoint.type';packages/mcp/mcp-connector/src/sse/sse-server-endpoint.ts (1)
1-3
: Fix import type consistency.Multiple imports are only used as types and should use
import type
for consistency.-import * as http from 'node:http'; -import { JSONRPCMessage } from '@modelcontextprotocol/sdk/types'; -import { EndpointMessageType, IConnectorEndpoint, IEndpointMessage } from '../endpoint.type'; +import * as http from 'node:http'; +import type { JSONRPCMessage } from '@modelcontextprotocol/sdk/types'; +import { EndpointMessageType } from '../endpoint.type'; +import type { IConnectorEndpoint, IEndpointMessage } from '../endpoint.type';
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
demo-server/src/connector.ts
(2 hunks)demo/.env.development
(1 hunks)demo/src/mcp/index.js
(2 hunks)docs/src/extensions/connector.md
(1 hunks)packages/mcp/mcp-connector/src/sse/sse-client-endpoint.ts
(1 hunks)packages/mcp/mcp-connector/src/sse/sse-endpoint-server.ts
(1 hunks)packages/mcp/mcp-connector/src/sse/sse-server-endpoint.ts
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- demo/.env.development
🚧 Files skipped from review as they are similar to previous changes (1)
- demo/src/mcp/index.js
🧰 Additional context used
🧬 Code Graph Analysis (2)
demo-server/src/connector.ts (3)
packages/mcp/mcp-connector/src/connector-center.ts (1)
ConnectorCenter
(4-22)packages/mcp/mcp-connector/src/sse/sse-server-endpoint.ts (1)
SSEServerEndpoint
(5-55)packages/mcp/mcp-connector/src/sse/sse-endpoint-server.ts (1)
SSEEndpointServer
(7-47)
packages/mcp/mcp-connector/src/sse/sse-client-endpoint.ts (1)
packages/mcp/mcp-connector/src/endpoint.type.ts (2)
IConnectorEndpoint
(13-25)IEndpointMessage
(7-11)
🪛 ESLint
demo-server/src/connector.ts
[error] 1-7: Imports "WebSocketServerEndpoint" and "SSEServerEndpoint" are only used as type.
(@typescript-eslint/consistent-type-imports)
[error] 3-3: Member 'ConnectorCenter' of the import declaration should be sorted alphabetically.
(sort-imports)
packages/mcp/mcp-connector/src/sse/sse-client-endpoint.ts
[error] 1-1: All imports in the declaration are only used as types. Use import type
.
(@typescript-eslint/consistent-type-imports)
[error] 2-2: Imports "IConnectorEndpoint" and "IEndpointMessage" are only used as type.
(@typescript-eslint/consistent-type-imports)
[error] 8-8: The 'EventSource' is still an experimental feature The configured version range is '^20.19.0 || >=22.12.0'.
(n/no-unsupported-features/node-builtins)
[error] 21-21: The 'EventSource' is still an experimental feature The configured version range is '^20.19.0 || >=22.12.0'.
(n/no-unsupported-features/node-builtins)
packages/mcp/mcp-connector/src/sse/sse-endpoint-server.ts
[error] 2-2: All imports in the declaration are only used as types. Use import type
.
(@typescript-eslint/consistent-type-imports)
packages/mcp/mcp-connector/src/sse/sse-server-endpoint.ts
[error] 1-1: All imports in the declaration are only used as types. Use import type
.
(@typescript-eslint/consistent-type-imports)
[error] 2-2: All imports in the declaration are only used as types. Use import type
.
(@typescript-eslint/consistent-type-imports)
[error] 3-3: Imports "IConnectorEndpoint" and "IEndpointMessage" are only used as type.
(@typescript-eslint/consistent-type-imports)
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.
Actionable comments posted: 0
♻️ Duplicate comments (4)
packages/mcp/mcp-connector/src/sse/sse-endpoint-server.ts (1)
33-35
: Add error handling and await the endpoint initialization.The endpoint creation and startup lacks proper error handling, and
endpoint.start()
is called without awaiting, which could lead to race conditions.Apply this diff to add proper error handling:
- endpoint.start(); - - this.connectorCenter.setClient(clientId, endpoint); + try { + await endpoint.start(); + this.connectorCenter.setClient(clientId, endpoint); + } catch (error) { + console.error('Failed to start SSE endpoint:', error); + res.statusCode = 500; + res.end('Internal Server Error'); + return; + }packages/mcp/mcp-connector/src/sse/sse-server-endpoint.ts (1)
18-53
: Fix Promise resolution and prevent memory leaks from event listeners.The
start()
method has two issues:
- It doesn't resolve the returned Promise, causing any
await endpoint.start()
to hang indefinitely- It adds a new event listener to the server for each endpoint instance, which can cause memory leaks
Apply this diff to fix both issues:
- async start(): Promise<void> { - this.app.on('request', (req, res) => { + private requestHandler = (req: http.IncomingMessage, res: http.ServerResponse) => { + if (req.method === 'OPTIONS' && req.url === '/message') { + res.writeHead(204, { + 'Access-Control-Allow-Origin': '*', + 'Access-Control-Allow-Methods': '*', + 'Access-Control-Allow-Headers': '*', + }); + return res.end(); + } + if (req.url === '/message') { + res.setHeader('Access-Control-Allow-Origin', '*'); + res.setHeader('Access-Control-Allow-Methods', '*'); + + let body = ''; + + req.on('data', (chunk) => { + body += chunk.toString(); + }); + + req.on('end', () => { + try { + const message = JSON.parse(body); + if (message.type === EndpointMessageType.INITIALIZE) { + res.end(); + return; + } + + this.onmessage?.(message); + } finally { + res.end(); + } + }); + } + }; + + async start(): Promise<void> { + this.app.on('request', this.requestHandler); + } + + async close(): Promise<void> { + this.app.removeListener('request', this.requestHandler); + this.res.end(); + }docs/src/extensions/connector.md (2)
122-122
: Fix undefined variable reference.The code references
config.port
but should referencethis.port
.- this.app.listen(config.port); + this.app.listen(this.port);
227-236
: Use standard URL API and return the fetch promise.In browsers
URL.parse
is not available; usenew URL(this.url)
. Also return thefetch
call to allow proper promise handling.- const url = URL.parse(this.url); - - // 请求/message api 向服务端发送请求 - fetch(`${url?.origin}/message`, { + const endpointUrl = new URL(this.url); + + // 请求/message api 向服务端发送请求 + return fetch(`${endpointUrl.origin}/message`, { method: 'POST', body: JSON.stringify(message), headers: { 'Content-Type': 'application/json', }, });
🧹 Nitpick comments (1)
packages/mcp/mcp-connector/src/sse/sse-server-endpoint.ts (1)
11-11
: Use proper typing for the response parameter.The
res
parameter should be typed ashttp.ServerResponse
instead ofany
for better type safety.- constructor(app: http.Server, res: any, clientId: string) { + constructor(app: http.Server, res: http.ServerResponse, clientId: string) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
docs/src/extensions/connector.md
(1 hunks)packages/mcp/mcp-connector/src/sse/sse-endpoint-server.ts
(1 hunks)packages/mcp/mcp-connector/src/sse/sse-server-endpoint.ts
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
packages/mcp/mcp-connector/src/sse/sse-endpoint-server.ts (2)
packages/mcp/mcp-connector/src/connector-center.ts (1)
ConnectorCenter
(4-22)packages/mcp/mcp-connector/src/sse/sse-server-endpoint.ts (1)
SSEServerEndpoint
(5-63)
packages/mcp/mcp-connector/src/sse/sse-server-endpoint.ts (1)
packages/mcp/mcp-connector/src/endpoint.type.ts (2)
IConnectorEndpoint
(13-25)IEndpointMessage
(7-11)
🪛 ESLint
packages/mcp/mcp-connector/src/sse/sse-endpoint-server.ts
[error] 2-2: All imports in the declaration are only used as types. Use import type
.
(@typescript-eslint/consistent-type-imports)
packages/mcp/mcp-connector/src/sse/sse-server-endpoint.ts
[error] 1-1: All imports in the declaration are only used as types. Use import type
.
(@typescript-eslint/consistent-type-imports)
[error] 2-2: All imports in the declaration are only used as types. Use import type
.
(@typescript-eslint/consistent-type-imports)
[error] 3-3: Imports "IConnectorEndpoint" and "IEndpointMessage" are only used as type.
(@typescript-eslint/consistent-type-imports)
🔇 Additional comments (2)
packages/mcp/mcp-connector/src/sse/sse-endpoint-server.ts (1)
1-1
: Static analysis hint is incorrect - runtime import is needed.The ESLint hint suggesting to use
import type
is incorrect here. Thehttp
module contains runtime values (likehttp.createServer()
) that are used in the implementation, not just types.packages/mcp/mcp-connector/src/sse/sse-server-endpoint.ts (1)
1-3
: Static analysis hints are incorrect - runtime imports are needed.The ESLint hints suggesting to use
import type
are incorrect. These imports include runtime values (http.Server
,JSONRPCMessage
) that are used in the implementation, not just type annotations.
|
||
const url = URL.parse(this.url); | ||
|
||
fetch(`${url?.origin}/message`, { |
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.
这块应该是配置项, 没有就用默认的message, 最好就是有只用配置项,强制需要配置
protected connectorCenter: ConnectorCenter<SSEServerEndpoint>; | ||
|
||
constructor(config: { port: number }, connectorCenter: ConnectorCenter<SSEServerEndpoint>) { | ||
this.app = http.createServer(); |
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.
需要允许noServer的配置 统一从外部走,这样可以走外部监听端口,以及重新配置url
this.app.on('request', (req, res) => { | ||
if (req.method === 'OPTIONS' && req.url === '/message') { | ||
res.writeHead(204, { | ||
'Access-Control-Allow-Origin': '*', |
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.
这些应该保留给外部配置?
extensions add sse endpoint
Summary by CodeRabbit
New Features
Documentation