-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Add HTTP headers support for MCP servers (#1609) #2007
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
Conversation
- Add headersJson field to mcp_servers schema - Update TypeScript types to include headersJson - Add HTTP Headers editor UI (reusing EnvVarsEditor component) - Pass headers to StreamableHTTPClientTransport via requestInit - Generate database migration for headers_json column Fixes dyad-sh#1609
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.
No issues found across 9 files
|
@BugBot run |
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.
✅ Bugbot reviewed your changes and found no bugs!
wwwillchen
left a comment
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.
Thanks for the PR!
Could you add an e2e test case? You can see our existing mcp test here: https://github.com/dyad-sh/dyad/blob/main/e2e-tests/mcp.spec.ts
info on e2e tests: https://github.com/dyad-sh/dyad/blob/main/CONTRIBUTING.md#testing
if you can also rebase, i'll kick off the CI run. thanks!
| {s.transport === "http" && ( | ||
| <div className="mt-3"> | ||
| <div className="text-sm font-medium mb-2">Headers</div> | ||
| <EnvVarsEditor |
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.
Let's rename this component to KeyValueEditor since we're using it for both env and headers.
| <div className="text-sm font-medium mb-2">Headers</div> | ||
| <EnvVarsEditor | ||
| serverId={s.id} | ||
| envJson={s.headersJson} |
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.
just call this json
| <div className="mt-3"> | ||
| <div className="text-sm font-medium mb-2">Headers</div> | ||
| <EnvVarsEditor | ||
| serverId={s.id} |
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.
just id?
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.
I will update PR shortly with the suggested changes
Greptile SummaryThis PR adds support for custom HTTP headers to HTTP-based MCP servers, enabling authentication and custom header configuration. The implementation follows the repository's established patterns for IPC, database schema, and UI components. Key Changes:
Implementation Quality: Confidence Score: 5/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant UI as ToolsMcpSettings
participant IPC as IpcClient
participant Handler as mcp_handlers
participant DB as Database
participant Manager as McpManager
participant Transport as StreamableHTTPClientTransport
participant Server as HTTP MCP Server
User->>UI: Configure HTTP MCP server
User->>UI: Add Authorization header
UI->>UI: Parse headers with arrayToJsonObject
UI->>IPC: updateServer({id, headersJson})
IPC->>Handler: mcp:update-server
Handler->>DB: UPDATE mcp_servers SET headers_json
DB-->>Handler: Success
Handler-->>IPC: Updated server
IPC-->>UI: Success
UI->>User: Show success toast
Note over User,Server: Later: Using the MCP server
User->>UI: Send prompt requiring tool
UI->>Manager: getClient(serverId)
Manager->>DB: SELECT * FROM mcp_servers WHERE id
DB-->>Manager: Server config with headersJson
Manager->>Manager: Extract headers from headersJson
Manager->>Transport: new StreamableHTTPClientTransport(url, {requestInit: {headers}})
Transport-->>Manager: Transport instance
Manager->>Transport: experimental_createMCPClient({transport})
Transport->>Server: POST /message (with Authorization header)
Server-->>Transport: JSON-RPC response
Transport-->>Manager: MCP client
Manager-->>UI: Client ready
UI->>User: Execute tool call
|
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.
Additional Comments (1)
-
src/db/schema.ts, line 219-222 (link)logic: Migration file deleted in commit
0cc53d1. Theheaders_jsoncolumn requires a migration (drizzle/0019_thin_union_jack.sqlwas removed). Runnpm run db:generateand restore the migration, or users will encounter runtime errors when the schema doesn't match the database.
5 files reviewed, 1 comment
Greptile found no issues!From now on, if a review finishes and we haven't found any issues, we will not post anything, but you can confirm that we reviewed your changes in the status check section. This feature can be toggled off in your Code Review Settings by deselecting "Create a status check for each PR". |
|
@BugBot run |
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.
✅ Bugbot reviewed your changes and found no bugs!
🎭 Playwright Test Results❌ Some tests failed
Summary: 459 passed, 4 failed, 3 flaky, 150 skipped Failed Tests🍎 macOS
🪟 Windows
|
| await po.selectChatMode("agent"); | ||
| // Wait for chat input to be ready | ||
| await po.getChatInput().waitFor({ state: "visible" }); | ||
| await po.sendPrompt("[call_tool=calculator_add_2]", { |
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.
i don't think this e2e test is actually calling the MCP server - i don't see the MCP consent dialog being shown when i run it locally.
see
| lastMessage.content.includes("[call_tool=calculator_add]") |
Fixes #1609
Summary by cubic
Add per-server HTTP headers for HTTP-based MCP servers so requests can include auth or custom headers. Includes settings UI, schema updates, and runtime support; fixes #1609.
New Features
Migration
Written for commit 15ffed8. Summary will update on new commits.
Note
Introduces configurable HTTP headers for HTTP-transport MCP servers, wired end-to-end across DB, IPC, UI, and runtime.
headers_jsontomcp_servers(migration0021), update Drizzle schema and IPC types/handlers to persist and update headersStreamableHTTPClientTransportviarequestInit.headersKeyValueEditor; add "Headers" editor for HTTP servers and keep env vars forstdioAuthorization; add server script, README, and snapshotWritten by Cursor Bugbot for commit 15ffed8. This will update automatically on new commits. Configure here.