feat: 使 chatluna agent 可以单独配置代理#871
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughMCP 传输层扩展为支持通过 ChatLunaPlugin 的代理功能： ChangesMCP Transport 代理功能与 Service 集成
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR updates MCP transport creation to route HTTP/SSE requests through the ChatLuna plugin (enabling proxy-aware fetch), and adjusts tool invocation typing at the LangChain tool boundary.
Changes:
- Pass the ChatLuna plugin into
createTransport()so transports can useplugin.fetch(with proxy support). - Inject a proxy-aware
fetchimplementation into MCP transport options for non-stdio transports. - Loosen the LangChain tool input type to
unknownand cast when callingcallTool.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| packages/extension-agent/src/service/mcp.ts | Passes plugin into transport creation; changes tool input typing and casts input to expected shape for callTool. |
| packages/extension-agent/src/mcp/transport.ts | Extends createTransport API to accept plugin and wires plugin.fetch(..., config.proxy) into transport config via a custom fetch. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| async (input: unknown) => { | ||
| return await callTool( | ||
| serverName, | ||
| mcpTool.name, | ||
| client, | ||
| input, | ||
| input as Record<string, unknown>, | ||
| { timeout: t.timeout }, |
| export function createTransport( | ||
| name: string, | ||
| config: McpServerConfig | ||
| config: McpServerConfig, | ||
| plugin: ChatLunaPlugin | ||
| ): Transport { |
| const proxyFetch: typeof fetch = (url, init) => { | ||
| return plugin.fetch( | ||
| url as Parameters<typeof plugin.fetch>[0], | ||
| init as Parameters<typeof plugin.fetch>[1], | ||
| config.proxy | ||
| ) as unknown as ReturnType<typeof fetch> | ||
| } |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces proxy support for MCP transports by integrating ChatLunaPlugin.fetch into the transport configuration and updating the createTransport function signature. Feedback suggests optimizing the transport setup by conditionally defining the fetch property only when necessary (e.g., for SSE or HTTP) to avoid redundant logic for stdio transports. Additionally, it is recommended to implement runtime type checks for tool inputs to maintain safety after changing the input type to unknown and using a type assertion.
| const proxyFetch: typeof fetch = (url, init) => { | ||
| return plugin.fetch( | ||
| url as Parameters<typeof plugin.fetch>[0], | ||
| init as Parameters<typeof plugin.fetch>[1], | ||
| config.proxy | ||
| ) as unknown as ReturnType<typeof fetch> | ||
| } | ||
|
|
||
| const transportConfig = { | ||
| ...config, | ||
| requestInit: { | ||
| ...requestInit, | ||
| headers | ||
| } | ||
| }, | ||
| fetch: proxyFetch | ||
| } |
There was a problem hiding this comment.
proxyFetch 的定义和 transportConfig 的构建逻辑目前在所有传输类型下都会执行。然而，对于 stdio 类型的传输，这些逻辑（特别是 fetch 选项）是多余的。建议将这部分逻辑移动到 if (type === 'stdio') 判断之后，或者通过条件判断仅在需要时（如 sse 或 http）才定义 fetch 属性，以优化执行效率并提升代码的可读性。
| async (input: unknown) => { | ||
| return await callTool( | ||
| serverName, | ||
| mcpTool.name, | ||
| client, | ||
| input, | ||
| input as Record<string, unknown>, |
There was a problem hiding this comment.
将 input 的类型从 Record<string, unknown> 改为 unknown 并配合类型断言 as Record<string, unknown> 会削弱类型安全性。虽然这可能是为了解决 langchain 工具定义时的类型推导问题，但建议在调用 callTool 之前增加简单的类型检查（例如 if (typeof input === 'object' && input !== null)），以防止非对象输入导致运行时错误。
No description provided.