LCORE-1350: Add server url check#1231
Conversation
WalkthroughThe pull request refines header construction logic in the responses utility module. MCP headers are now generated exclusively from tools that have both headers defined and a non-empty server URL. Additionally, documentation for a function parameter type is clarified. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes 🚥 Pre-merge checks | ✅ 3✅ 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.
🧹 Nitpick comments (1)
src/utils/responses.py (1)
186-190: Add a focused regression test for server URL gating.The new filter on Line 189 is good, but this path should be covered with explicit tests (include/exclude cases for MCP tools with headers + empty/non-empty
server_url) to avoid future regressions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils/responses.py` around lines 186 - 190, Add a focused regression test that verifies the server_url gating in the mcp_headers comprehension: exercise cases where tools contain entries with type == "mcp" with headers present and server_url non-empty (should be included in mcp_headers), with headers present but server_url empty/None (should be excluded), and with non-mcp types (should be excluded). Create tests that build a small list named tools matching the structure used by the mcp_headers comprehension and assert the resulting mcp_headers mapping contains only the expected tool.server_url -> tool.headers pairs; include both include and exclude assertions to prevent regressions around the tool.type, tool.headers, and tool.server_url checks.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/utils/responses.py`:
- Around line 186-190: Add a focused regression test that verifies the
server_url gating in the mcp_headers comprehension: exercise cases where tools
contain entries with type == "mcp" with headers present and server_url non-empty
(should be included in mcp_headers), with headers present but server_url
empty/None (should be excluded), and with non-mcp types (should be excluded).
Create tests that build a small list named tools matching the structure used by
the mcp_headers comprehension and assert the resulting mcp_headers mapping
contains only the expected tool.server_url -> tool.headers pairs; include both
include and exclude assertions to prevent regressions around the tool.type,
tool.headers, and tool.server_url checks.
Description
This PR adds additional server url check and fixes obsolete docstring.
Type of change
Tools used to create PR
Identify any AI code assistants used in this PR (for transparency and review context)
Related Tickets & Documents
Checklist before requesting a review
Testing
Summary by CodeRabbit