fix: override empty cookie header in MCP header forwarding hook

[sarah-inkeep]

No description provided.

[changeset-bot]

🦋 Changeset detected

Latest commit: a4369c5

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 9 packages

Name	Type
@inkeep/agents-manage-mcp	Patch
@inkeep/agents-manage-api	Patch
@inkeep/agents-run-api	Patch
@inkeep/agents-manage-ui	Patch
@inkeep/agents-cli	Patch
@inkeep/agents-core	Patch
@inkeep/agents-sdk	Patch
@inkeep/ai-sdk-provider	Patch
@inkeep/create-agents	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Review	Updated (UTC)
agents-docs	Ready	Preview, Comment	Jan 6, 2026 9:19pm
agents-manage-api	Ready	Preview, Comment	Jan 6, 2026 9:19pm
agents-manage-ui	Ready	Preview, Comment	Jan 6, 2026 9:19pm
agents-run-api	Ready	Preview, Comment	Jan 6, 2026 9:19pm

[claude]

Claude finished @sarah-inkeep's task —— View job

---

Code Review Complete

Todo List:

• Examine the modified file and understand the specific changes
• Research existing header forwarding patterns in the codebase
• Analyze the MCP implementation context
• Review the cookie header fix for correctness and completeness
• Evaluate system design and architectural considerations
• Provide comprehensive review feedback

---

Review Summary

This PR fixes a specific issue where empty cookie headers set by the SDK were not being properly overridden by the HeaderForwardingHook. The change is minimal, targeted, and addresses a real authentication flow problem.

Technical Analysis

Root Cause Identified:
The issue stems from the SDK's cookie handling in sdks.ts:173-178, which automatically sets an empty cookie header (cookie: "") by default. The previous logic in HeaderForwardingHook only checked for header existence using newHeaders.has(key), which returned true for empty cookie headers, preventing forwarded cookies from being applied.

Fix Analysis:

// Before: Only checked existence
if (!newHeaders.has(key)) {
  newHeaders.set(key, value);
}

// After: Checks value and special-cases empty cookies
const existingValue = newHeaders.get(key);
if (!existingValue || (key === 'cookie' && existingValue === '')) {
  newHeaders.set(key, value);
}

Engineering Assessment

✅ Strengths:

1. Minimal, Surgical Fix: The change precisely targets the issue without affecting other headers or system behavior
2. Clear Intent: The comment explains the SDK behavior and differentiated handling strategy
3. Backward Compatible: Preserves existing behavior for all non-cookie headers
4. Performance: Efficient - only calls get() once per header instead of both has() and get()

⚠️ Areas for Consideration:

1. Missing Tests: There are no unit tests for this critical authentication hook. The fix addresses a real bug that could silently break authentication flows.

2. Magic String: The special-case logic uses a hardcoded 'cookie' string. Consider using a constant for better maintainability:

   const COOKIE_HEADER = 'cookie';
   // ...
   if (!existingValue || (key === COOKIE_HEADER && existingValue === '')) {

3. Case Sensitivity: HTTP headers are case-insensitive, but this comparison is case-sensitive. While cookie is the standard lowercase form, consider defensive programming:

   if (!existingValue || (key.toLowerCase() === 'cookie' && existingValue === '')) {

4. Documentation Gap: The behavior difference between cookie and non-cookie headers could benefit from JSDoc clarification:

   /**
    * HeaderForwardingHook forwards specified headers from incoming requests to outgoing SDK requests.
    * 
    * Special behavior:
    * - Cookie headers: Overrides empty cookies (SDK sets empty cookie by default)
    * - Other headers: Only sets if not already present
    */

System Architecture Considerations

1. Authentication Flow Integrity: This fix is critical for MCP server authentication. Consider adding integration tests that verify end-to-end cookie forwarding.

2. Hook Composition: The differentiated behavior between cookie and other headers is reasonable given the SDK's default behavior, but monitor for similar issues with other headers.

3. Error Boundaries: Consider logging when cookie override occurs for debugging authentication issues.

Recommendation

Approve with suggestions - The core fix is sound and addresses a real authentication bug. Consider implementing the suggested improvements in follow-up work:

1. Add unit tests for the hook covering both cookie and non-cookie header scenarios
2. Add integration tests for authentication flow
3. Consider the case sensitivity and magic string improvements
4. Update JSDoc documentation

The fix correctly addresses the immediate issue and maintains system stability while preserving backward compatibility.