fix: avoid duplicate Content-Type header in fetch402#50
Conversation
When a body is passed it is sent as JSON, so we default the Content-Type header. The agent isn't required to set the header itself but might do so anyway (in any casing), which previously produced two Content-Type headers. Only add our default when one isn't already present. Fixes #47 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe ChangesHeader deduplication fix
Estimated code review effort: 1 (Trivial) | ~3 minutes Related issues: Fixes duplicate Suggested reviewers: rolznz 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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.
🧹 Nitpick comments (1)
src/tools/lightning/fetch.ts (1)
25-32: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueConsider generalizing header deduplication, if broader
fetch402Lib/undici header merging is a concern.This fix only guards against a duplicate
Content-Type. The linked issue notes the fix "should ideally be handled as a general header deduplication improvement" for other headers too. Sincefetch402Lib's handling ofrequestOptions.headersdownstream isn't shown here, if it merges headers again (e.g., with its own defaults for other header names), the same duplicate-value issue could reappear for headers other thanContent-Type. Given the issue explicitly scoped this as a possible follow-up rather than a blocking requirement, this is optional.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/tools/lightning/fetch.ts` around lines 25 - 32, The current fix in fetch.ts only de-dupes Content-Type before assigning requestOptions.headers, but the review notes this may need to be generalized if fetch402Lib or undici merges other headers downstream. Update the header handling around params.headers/requestOptions.headers so duplicate values are prevented for all header names, not just Content-Type, while keeping the existing behavior intact in the fetch path.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/tools/lightning/fetch.ts`:
- Around line 25-32: The current fix in fetch.ts only de-dupes Content-Type
before assigning requestOptions.headers, but the review notes this may need to
be generalized if fetch402Lib or undici merges other headers downstream. Update
the header handling around params.headers/requestOptions.headers so duplicate
values are prevented for all header names, not just Content-Type, while keeping
the existing behavior intact in the fetch path.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 67235fd1-e318-465e-be35-07efdcd3aadb
📒 Files selected for processing (1)
src/tools/lightning/fetch.ts
Summary
When a body is passed to
fetch402, it is sent as JSON so we default theContent-Typeheader. The agent isn't required to set the header itself but might do so anyway (potentially in different casing), which previously produced twoContent-Typeheaders via object spread.This only adds our default
Content-Type: application/jsonwhen one isn't already present (matched case-insensitively).Fixes #47
🤖 Generated with Claude Code
Summary by CodeRabbit
GET/HEADcalls so JSON requests now set a defaultContent-Type: application/jsononly when one isn’t already provided.Content-Typeentries.