feat: add HttpRequestTool for REST API testing#1195
Conversation
- New tool to make HTTP requests (GET, POST, PUT, PATCH, DELETE, HEAD, OPTIONS) - Supports custom headers, query params, request body - Includes timeout handling with AbortController - Follows redirect handling P2 fix: add setCleanupTimeout helper to cleanupRegistry for cleanup-safe timers
jatmn
left a comment
There was a problem hiding this comment.
Findings
-
[P1] Register the HTTP tool so users can actually call it
src/tools.ts:182
The PR addsHttpRequestTool, but it never imports it intosrc/tools.tsor adds it togetAllBaseTools(). That registry is whatgetTools()uses for the CLI, SDK, default preset, permission filtering, and ToolSearch exposure, so this ships new files and tests but no user-visibleHttpRequesttool. Please wire the tool into the base tool list, and decide whether it should be immediately available or deferred likeWebFetchTool. -
[P2] Re-save the new TypeScript files as UTF-8 text
src/tools/HttpRequestTool/HttpRequestTool.ts:1
The newHttpRequestToolsource, test, and prompt files are committed as UTF-16 LE, so Git treats them as binary (git diff --numstatreports- -for all three files andgh pr diffcannot show their contents). That makes the code hard to review in GitHub and can break normal text tooling. Please re-save these files as UTF-8, matching the rest of the repo, before merging. -
[P2] Enforce the response size limit while reading, not after
resp.text()
src/tools/HttpRequestTool/HttpRequestTool.ts:83
The prompt says the response size is limited to prevent memory issues, but the implementation first reads the entire response withawait resp.text()and only then truncates toMAX_BODY_CHARS. A large or streaming response can still consume unbounded memory before truncation happens. Please read fromresp.bodywith a cap, abort once the limit is reached, and add a focused test that proves an oversized response is stopped before the full body is buffered.
|
Thanks for putting this together — a first-class HTTP tool is a genuinely useful addition, and the API surface you've designed reads cleanly. I went through it independently alongside @jatmn's review, and his points all hold: the tool isn't yet wired into One more item worth raising before the next pass, since it's the most important one: No rush — once the registration, encoding, buffering, and the SSRF guard are addressed, I'd be glad to take another look. Appreciate the work here. |
Summary
Impact
Testing
Notes