fix(agent): web_fetch returns the real body; zero-byte writes fail loudly - #803
fix(agent): web_fetch returns the real body; zero-byte writes fail loudly#803sweetmantech wants to merge 1 commit into
Conversation
…udly
webFetchTool captured the response via bash process substitution
(-o >(head -c N >&3) with fd-3 juggling). That is a bash-ism and does not
expand in the sandbox's exec shell, so curl wrote to a file literally named
">(head -c 10000 >&3)", failed, and exited 23 with zero bytes. The exit-23
branch then mapped that to { success: true, truncated: true }, so every URL
came back as an empty-bodied 200 and agents concluded the web was blocked.
- Write the body to a mktemp file; stdout contract is `status\nbyteSize\n<body>`.
- truncated is derived from the real byte size, not from curl's exit code.
- A nonzero exit that produced no bytes now returns success: false with stderr.
Verified under POSIX sh (the sandbox exec shell): the new command returns
exit 0 / status 200 / 559 bytes for example.com, while the old one fails with
"syntax error near unexpected token `('".
chat#1918
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthrough
ChangesWeb fetch response handling
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ❌ 3❌ Failed checks (3 warnings)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
lib/agent/tools/webFetchTool.ts (1)
81-96: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSplit the command construction and output parsing from
execute.The
executecallback is 88 lines (Lines 50-137), exceeding the 20-line limit and combining argument construction, shell protocol generation, and response parsing. Extract focused helpers such asbuildFetchCommandandparseFetchOutput.As per coding guidelines, “Flag functions longer than 20 lines” and “Keep functions small and focused.”
🤖 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 `@lib/agent/tools/webFetchTool.ts` around lines 81 - 96, Refactor the oversized execute callback by extracting argument/command construction into a focused buildFetchCommand helper and stdout protocol parsing into parseFetchOutput. Keep execute responsible only for invoking the command and coordinating the existing fetch response and error behavior, while preserving the status\nbyteSize\nbody contract and temporary-file handling.Source: Coding guidelines
🤖 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.
Inline comments:
In `@lib/agent/tools/webFetchTool.ts`:
- Line 93: Align the web fetch tool’s public description and naming around the
byte-based truncation performed by `head -c` and `MAX_BODY_LENGTH`. Update the
description near the command construction to state a byte limit, or replace the
truncation with character-safe handling; keep the configured cap and
returned-body behavior consistent.
- Around line 117-123: Update the fetch result handling around the curl
exit-code check so every nonzero result.exitCode returns success: false,
regardless of size or partial response content. Preserve the existing error
message and ensure successful responses are only returned when curl exits with
code zero.
---
Nitpick comments:
In `@lib/agent/tools/webFetchTool.ts`:
- Around line 81-96: Refactor the oversized execute callback by extracting
argument/command construction into a focused buildFetchCommand helper and stdout
protocol parsing into parseFetchOutput. Keep execute responsible only for
invoking the command and coordinating the existing fetch response and error
behavior, while preserving the status\nbyteSize\nbody contract and
temporary-file handling.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 54718a5d-5707-48ee-9802-acfb8cb56b25
⛔ Files ignored due to path filters (1)
lib/agent/tools/__tests__/webFetchTool.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**
📒 Files selected for processing (1)
lib/agent/tools/webFetchTool.ts
| "printf '\\n%s' \"$status\"", | ||
| 'size=$(wc -c < "$tmp" | tr -d " ")', | ||
| 'printf \'%s\\n%s\\n\' "$status" "$size"', | ||
| `head -c ${MAX_BODY_LENGTH} "$tmp"`, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Align the documented character cap with the byte-based implementation.
head -c caps bytes, while the tool description promises MAX_BODY_LENGTH characters. Non-ASCII bodies can be truncated early or split mid-character. Define the limit as bytes in the public description/name, or apply a character-safe cap.
🤖 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 `@lib/agent/tools/webFetchTool.ts` at line 93, Align the web fetch tool’s
public description and naming around the byte-based truncation performed by
`head -c` and `MAX_BODY_LENGTH`. Update the description near the command
construction to state a byte limit, or replace the truncation with
character-safe handling; keep the configured cap and returned-body behavior
consistent.
| if (result.exitCode !== 0 && size === 0) { | ||
| return { | ||
| success: false, | ||
| error: `Fetch failed: ${result.stderr || result.stdout || "Unknown error"}`, | ||
| error: `Fetch failed (exit ${result.exitCode}): ${ | ||
| result.stderr || "no response body written" | ||
| }`, | ||
| }; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Treat every nonzero curl exit as a failed fetch.
A timeout or transport failure can write a partial body before curl exits nonzero. This branch currently returns success: true whenever size > 0, without indicating that the response is incomplete. With output redirected to a temp file, exit 23 is no longer the expected truncation mechanism.
Proposed fix
- if (result.exitCode !== 0 && size === 0) {
+ if (result.exitCode !== 0) {
return {
success: false,
error: `Fetch failed (exit ${result.exitCode}): ${
result.stderr || "no response body written"
}`,
};
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (result.exitCode !== 0 && size === 0) { | |
| return { | |
| success: false, | |
| error: `Fetch failed: ${result.stderr || result.stdout || "Unknown error"}`, | |
| error: `Fetch failed (exit ${result.exitCode}): ${ | |
| result.stderr || "no response body written" | |
| }`, | |
| }; | |
| if (result.exitCode !== 0) { | |
| return { | |
| success: false, | |
| error: `Fetch failed (exit ${result.exitCode}): ${ | |
| result.stderr || "no response body written" | |
| }`, | |
| }; |
🤖 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 `@lib/agent/tools/webFetchTool.ts` around lines 117 - 123, Update the fetch
result handling around the curl exit-code check so every nonzero result.exitCode
returns success: false, regardless of size or partial response content. Preserve
the existing error message and ensure successful responses are only returned
when curl exits with code zero.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
3 issues found across 2 files
Confidence score: 2/5
- In
lib/agent/tools/webFetchTool.ts, download limiting is applied only aftercurlfinishes writing, so very large responses can fill the sandbox disk before truncation and destabilize tool execution — enforce a hard byte cap during transfer and preserve a cleartruncatedsignal. - In
lib/agent/tools/webFetchTool.ts, treating partial files from interrupted/nonzero-exitcurlruns as successful can return incomplete content as if it were valid, which can mislead downstream agent decisions — gate success on zero exit status or return an explicit partial/error state. - In
lib/agent/tools/webFetchTool.ts, byte-based truncation (head -c) can split multibyte UTF-8 characters ifMAX_BODY_LENGTHis documented as character-based, leading to malformed text in responses — align docs to byte semantics or switch to character-safe truncation.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="lib/agent/tools/webFetchTool.ts">
<violation number="1" location="lib/agent/tools/webFetchTool.ts:66">
P1: Large responses can exhaust sandbox disk because truncation happens only after curl has fully written the temp file. Bound bytes while downloading, while retaining a reliable truncated indicator.</violation>
<violation number="2" location="lib/agent/tools/webFetchTool.ts:93">
P3: `head -c ${MAX_BODY_LENGTH} "$tmp"` truncates by bytes, but if the tool's description documents `MAX_BODY_LENGTH` as a character limit, non-ASCII response bodies can be truncated mid-character, producing invalid UTF-8 in the returned body. Consider clarifying the limit as byte-based in the docs, or using a character-safe truncation approach.</violation>
<violation number="3" location="lib/agent/tools/webFetchTool.ts:117">
P1: Interrupted/failed transfers that wrote a partial body are now reported as successful responses. Treat nonzero curl exits as failures (or explicitly expose partial/error state) rather than using byte count as the success criterion.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| String(Math.ceil(FETCH_TIMEOUT_MS / 1000)), | ||
| "-o", | ||
| `>(head -c ${MAX_BODY_LENGTH} >&3)`, | ||
| '"$tmp"', |
There was a problem hiding this comment.
P1: Large responses can exhaust sandbox disk because truncation happens only after curl has fully written the temp file. Bound bytes while downloading, while retaining a reliable truncated indicator.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/agent/tools/webFetchTool.ts, line 66:
<comment>Large responses can exhaust sandbox disk because truncation happens only after curl has fully written the temp file. Bound bytes while downloading, while retaining a reliable truncated indicator.</comment>
<file context>
@@ -63,7 +63,7 @@ USAGE:
String(Math.ceil(FETCH_TIMEOUT_MS / 1000)),
"-o",
- `>(head -c ${MAX_BODY_LENGTH} >&3)`,
+ '"$tmp"',
"-w",
shellEscape("%{http_code}"),
</file context>
| // A nonzero exit that produced no bytes is a hard failure. Reporting it as | ||
| // an empty-bodied 200 is what made agents conclude the whole web was | ||
| // unreachable; surface it so they retry or report instead. | ||
| if (result.exitCode !== 0 && size === 0) { |
There was a problem hiding this comment.
P1: Interrupted/failed transfers that wrote a partial body are now reported as successful responses. Treat nonzero curl exits as failures (or explicitly expose partial/error state) rather than using byte count as the success criterion.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/agent/tools/webFetchTool.ts, line 117:
<comment>Interrupted/failed transfers that wrote a partial body are now reported as successful responses. Treat nonzero curl exits as failures (or explicitly expose partial/error state) rather than using byte count as the success criterion.</comment>
<file context>
@@ -96,25 +101,34 @@ USAGE:
+ // A nonzero exit that produced no bytes is a hard failure. Reporting it as
+ // an empty-bodied 200 is what made agents conclude the whole web was
+ // unreachable; surface it so they retry or report instead.
+ if (result.exitCode !== 0 && size === 0) {
return {
success: false,
</file context>
| if (result.exitCode !== 0 && size === 0) { | |
| if (result.exitCode !== 0) { |
| "printf '\\n%s' \"$status\"", | ||
| 'size=$(wc -c < "$tmp" | tr -d " ")', | ||
| 'printf \'%s\\n%s\\n\' "$status" "$size"', | ||
| `head -c ${MAX_BODY_LENGTH} "$tmp"`, |
There was a problem hiding this comment.
P3: head -c ${MAX_BODY_LENGTH} "$tmp" truncates by bytes, but if the tool's description documents MAX_BODY_LENGTH as a character limit, non-ASCII response bodies can be truncated mid-character, producing invalid UTF-8 in the returned body. Consider clarifying the limit as byte-based in the docs, or using a character-safe truncation approach.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/agent/tools/webFetchTool.ts, line 93:
<comment>`head -c ${MAX_BODY_LENGTH} "$tmp"` truncates by bytes, but if the tool's description documents `MAX_BODY_LENGTH` as a character limit, non-ASCII response bodies can be truncated mid-character, producing invalid UTF-8 in the returned body. Consider clarifying the limit as byte-based in the docs, or using a character-safe truncation approach.</comment>
<file context>
@@ -78,15 +78,20 @@ USAGE:
- "printf '\\n%s' \"$status\"",
+ 'size=$(wc -c < "$tmp" | tr -d " ")',
+ 'printf \'%s\\n%s\\n\' "$status" "$size"',
+ `head -c ${MAX_BODY_LENGTH} "$tmp"`,
+ 'rm -f "$tmp"',
"exit $curlExit",
</file context>
1 participant
Closes the
web_fetchrow of chat#1918. No docs change —web_fetchis an internal sandbox agent tool with no OpenAPI contract.The bug
webFetchTool.tsL66 captured the response body with bash process substitution:That does not expand in the sandbox's exec shell. curl writes to a file literally named
>(head -c 10000 >&3), fails, and exits 23. L100 then whitelisted exit 23 as "expected truncation" and L117 settruncated: true, so a hard write failure was reported as{ success: true, status: 200, body: "", truncated: true }.Every URL returned an empty body with a plausible status. Agents concluded the whole internet was unreachable — including
docs.recoupable.dev, so they could not even discover the API surface.Proof, under the actual shell
New command:
Old command, same shell:
Observed identically in a live sandbox run on 2026-07-30 (chat
329c4760-646b-4749-a7e7-82fd64f9cf72): bashcurl https://example.comreturned HTTP 200 / 559 bytes, whileweb_fetchon the same URL returned{"body": "", "status": 200, "success": true, "truncated": true}.The fix
mktempfile. stdout contract isstatus\nbyteSize\n<body>— the real size survives even though the body is capped atMAX_BODY_LENGTHfor context.truncatedis derived from the byte size, not curl's exit code.success: falsewith stderr, instead of a fake 200.Tests
RED→GREEN, 7 tests in
lib/agent/tools/__tests__/webFetchTool.test.ts(5 were RED before the change):The previous test suite asserted the buggy contract (
exit 23 → success: true); that assertion is replaced.pnpm exec vitest run lib/agent→ 50 files, 310 tests, all passpnpm exec eslinton both files → cleanpnpm exec tsc --noEmit→ the 3TS2769errors onwebFetchTool.tsL48-50 are pre-existing onmain(verified by stashing: identical lines and count), untouched by this changeRemaining verification
Done-when in the issue also asks for confirmation inside a real sandbox run. The local POSIX-sh repro above reproduces the failure and the fix exactly, but I have not yet exercised the deployed preview — worth doing before merge.
🤖 Generated with Claude Code
Summary by cubic
Fixes the
web_fetchsandbox tool so it returns real response bodies and treats zero-byte writes as failures. Resolves the regression where every URL appeared as an empty 200 (chat#1918).mktempand emitstatus\nbyteSize\n<body>on stdout.truncatedfrom byte size, not curl exit code.success: falseon any nonzero exit when size is 0; include stderr.Written for commit 0d51a4c. Summary will update on new commits.
Summary by CodeRabbit