feat: headers override for MCP operations#253
Conversation
WalkthroughThe pull request introduces a new "Headers Override" parameter to the McpClient node for SSE and HTTP connections, enabling users to add or override request headers. Header handling is refactored by extracting Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Node as McpClient Node
participant Utils as Header Utils
participant Transport as Transport Setup
User->>Node: Provide credentials + headers override
rect rgb(200, 220, 240)
Note over Node,Utils: Header Processing
Node->>Utils: parseHeaders(credentialHeaders)
Utils-->>Node: parsedCredentials
Node->>Utils: parseHeaders(headersOverride)
Utils-->>Node: parsedOverride
end
rect rgb(220, 240, 200)
Note over Node,Utils: Header Merging
Node->>Utils: mergeHeaders(parsedCredentials, parsedOverride)
Utils-->>Node: mergedHeaders<br/>(override takes precedence)
end
Node->>Transport: Setup with mergedHeaders
Transport-->>User: Request ready with combined headers
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
nodes/McpClient/utils.ts (1)
4-22: Consider simplifying the value check.The condition
value !== undefinedat line 15 is redundant becausesubstring()always returns a string, neverundefined. If you want to allow empty header values (e.g.,"Key="), you can simplify to just checkname. If you want to exclude empty values, checkname && valueinstead.Apply this diff if you want to allow empty values:
const name = line.substring(0, equalsIndex).trim(); const value = line.substring(equalsIndex + 1).trim(); // Add to headers object if key is not empty and value is defined - if (name && value !== undefined) { + if (name) { headers[name] = value; }Or this diff if you want to exclude empty values:
const name = line.substring(0, equalsIndex).trim(); const value = line.substring(equalsIndex + 1).trim(); - // Add to headers object if key is not empty and value is defined - if (name && value !== undefined) { + // Add to headers object if key and value are not empty + if (name && value) { headers[name] = value; }nodes/McpClient/__tests__/utils.test.ts (1)
4-55: Consider adding a test for empty header values.The test suite is comprehensive, but it doesn't explicitly test the case where a header has an empty value (e.g.,
"Authorization=\nKey=value"). While the current implementation would handle this, explicit test coverage would make the intended behavior clearer.Consider adding a test like:
it('should handle headers with empty values', () => { const input = 'Authorization=\nContent-Type=application/json\nX-Empty='; const result = parseHeaders(input); expect(result).toEqual({ Authorization: '', 'Content-Type': 'application/json', 'X-Empty': '', }); });nodes/McpClient/McpClient.node.ts (1)
364-379: Consider reusingparseHeadersfor environment variable parsing.The manual parsing logic here (lines 364-379) is very similar to the
parseHeadersutility function. While outside the scope of this PR, consider refactoring this to useparseHeadersin the future to reduce code duplication and improve maintainability.Example future refactor:
// Parse newline-separated environment variables from credentials if (cmdCredentials.environments) { const parsedEnv = parseHeaders(cmdCredentials.environments as string); Object.assign(env, parsedEnv); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
nodes/McpClient/McpClient.node.ts(4 hunks)nodes/McpClient/__tests__/utils.test.ts(1 hunks)nodes/McpClient/utils.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
nodes/McpClient/McpClient.node.ts (1)
nodes/McpClient/utils.ts (2)
parseHeaders(4-22)mergeHeaders(28-33)
nodes/McpClient/__tests__/utils.test.ts (1)
nodes/McpClient/utils.ts (2)
parseHeaders(4-22)mergeHeaders(28-33)
🔇 Additional comments (6)
nodes/McpClient/utils.ts (1)
28-32: LGTM!The merge implementation is clean and correctly implements the documented precedence behavior where dynamic headers override credential headers.
nodes/McpClient/__tests__/utils.test.ts (1)
57-136: LGTM!The test coverage for
mergeHeadersis thorough and validates all key scenarios including precedence behavior, empty inputs, and multiple header merging.nodes/McpClient/McpClient.node.ts (4)
15-15: LGTM!The import statement correctly references the new utility functions.
104-116: LGTM!The parameter definition is well-structured with appropriate display options limiting visibility to SSE and HTTP connection types, and the description clearly explains the expected format and merge precedence behavior.
265-278: LGTM!The header handling for HTTP transport correctly implements the feature with proper parsing of both credential and override headers, backward-compatible parameter retrieval, and correct merge precedence.
313-326: LGTM!The SSE transport implementation mirrors the HTTP path correctly, with headers properly applied to both
eventSourceInitandrequestInitas needed for SSE connections.
|
is there any chance to merge it? |
|
hey @iamfiscus , thanks for merging that. Although, publish to NPM registry failed -> token has expired. Could you sort it out? :-) |
5 participants
Description
Adds Headers Override functionality for MCP Client node, allowing dynamic header injection for HTTP and SSE connections. Override headers take precedence over credential headers, enabling per-workflow customization of API requests.
Related Issue
N/A
Type of Change
How Has This Been Tested?
parseHeadersandmergeHeadersutility functions (all passing)Checklist
Release Notes
New Feature: Headers Override
NAME=VALUEpairsScreenshots (if applicable)
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.