Skip to content

fix(agent): web_fetch returns the real body; zero-byte writes fail loudly #803

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
sweetmantech wants to merge 1 commit into
base: main
Choose a base branch
from
+103 −33
Open

fix(agent): web_fetch returns the real body; zero-byte writes fail loudly #803

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
88 changes: 72 additions & 16 deletions lib/agent/tools/__tests__/webFetchTool.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { describe, it, expect, vi, beforeEach } from "vitest";
import { webFetchTool } from "@/lib/agent/tools/webFetchTool";
import { webFetchTool, MAX_BODY_LENGTH } from "@/lib/agent/tools/webFetchTool";
import { connectVercel } from "@/lib/sandbox/vercel/connect/connectVercel";

vi.mock("@/lib/sandbox/vercel/connect/connectVercel", () => ({
Expand All @@ -12,18 +12,21 @@ function makeSandbox(exec: ReturnType<typeof vi.fn>) {
return { workingDirectory: "/sandbox/mono", exec };
}

/** The tool's stdout contract: `status\nbyteSize\n<body>`. */
function stdout(status: string, size: number, body: string) {
return `${status}\n${size}\n${body}`;
}

beforeEach(() => vi.clearAllMocks());

describe("webFetchTool", () => {
it("parses body + trailing status code on success", async () => {
// Body, then newline, then status code "200" (per the curl -w '%{http_code}' contract).
it("parses status, size and body on success", async () => {
const sb = makeSandbox(
vi.fn().mockResolvedValue({
success: true,
exitCode: 0,
stdout: '{"ok":true}\n200',
stdout: stdout("200", 11, '{"ok":true}'),
stderr: "",
truncated: false,
}),
);
vi.mocked(connectVercel).mockResolvedValue(sb as never);
Expand All @@ -38,32 +41,67 @@ describe("webFetchTool", () => {
});
});

it("marks truncated:true on curl exit 23 (head -c cut off the body)", async () => {
it("preserves a body that itself contains newlines", async () => {
const sb = makeSandbox(
vi.fn().mockResolvedValue({
success: false,
exitCode: 23,
stdout: "huge body fragment\n200",
success: true,
exitCode: 0,
stdout: stdout("200", 13, "line1\nline2\n"),
stderr: "",
}),
);
vi.mocked(connectVercel).mockResolvedValue(sb as never);
const result = (await webFetchTool.execute!({ url: "https://example.com/multiline" }, {
experimental_context: ctx,
} as never)) as { body: string };
expect(result.body).toBe("line1\nline2\n");
});

it("marks truncated by BYTE SIZE, not by exit code", async () => {
const big = MAX_BODY_LENGTH + 512;
const sb = makeSandbox(
vi.fn().mockResolvedValue({
success: true,
exitCode: 0,
stdout: stdout("200", big, "x".repeat(MAX_BODY_LENGTH)),
stderr: "",
truncated: false,
}),
);
vi.mocked(connectVercel).mockResolvedValue(sb as never);
const result = (await webFetchTool.execute!({ url: "https://example.com/huge" }, {
experimental_context: ctx,
} as never)) as { success: boolean; truncated: boolean };
} as never)) as { success: boolean; truncated: boolean; body: string };
expect(result.success).toBe(true);
expect(result.truncated).toBe(true);
expect(result.body.length).toBe(MAX_BODY_LENGTH);
});

it("returns success:false on non-0, non-23 curl exit", async () => {
// The regression this fix exists for (chat#1918): a zero-byte write must never be
// reported as a truncated success — that made agents believe every URL was empty.
it("returns success:false when curl exits 23 with an empty body", async () => {
const sb = makeSandbox(
vi.fn().mockResolvedValue({
success: false,
exitCode: 23,
stdout: stdout("200", 0, ""),
stderr: "curl: (23) Failure writing output to destination",
}),
);
vi.mocked(connectVercel).mockResolvedValue(sb as never);
const result = (await webFetchTool.execute!({ url: "https://example.com" }, {
experimental_context: ctx,
} as never)) as { success: boolean; error: string };
expect(result.success).toBe(false);
expect(result.error).toMatch(/23|writing output/i);
});

it("returns success:false on any nonzero exit with an empty body", async () => {
const sb = makeSandbox(
vi.fn().mockResolvedValue({
success: false,
exitCode: 7,
stdout: "",
stdout: stdout("000", 0, ""),
stderr: "Failed to connect",
truncated: false,
}),
);
vi.mocked(connectVercel).mockResolvedValue(sb as never);
Expand All @@ -74,14 +112,32 @@ describe("webFetchTool", () => {
expect(result.error).toMatch(/Failed to connect/);
});

it("does not use bash process substitution (unsupported in the sandbox shell)", async () => {
const sb = makeSandbox(
vi.fn().mockResolvedValue({
success: true,
exitCode: 0,
stdout: stdout("200", 2, "ok"),
stderr: "",
}),
);
vi.mocked(connectVercel).mockResolvedValue(sb as never);
await webFetchTool.execute!({ url: "https://example.com" }, {
experimental_context: ctx,
} as never);
const cmd = sb.exec.mock.calls[0]?.[0] as string;
expect(cmd).not.toContain(">(");
expect(cmd).not.toContain(">&3");
expect(cmd).toContain("mktemp");
});

it("passes the request body for POST", async () => {
const sb = makeSandbox(
vi.fn().mockResolvedValue({
success: true,
exitCode: 0,
stdout: "ok\n201",
stdout: stdout("201", 2, "ok"),
stderr: "",
truncated: false,
}),
);
vi.mocked(connectVercel).mockResolvedValue(sb as never);
Expand Down
48 changes: 31 additions & 17 deletions lib/agent/tools/webFetchTool.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ USAGE:
"--max-time",
String(Math.ceil(FETCH_TIMEOUT_MS / 1000)),
"-o",
`>(head -c ${MAX_BODY_LENGTH} >&3)`,
'"$tmp"',

@cubic-dev-ai cubic-dev-ai Bot Jul 31, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>

"-w",
shellEscape("%{http_code}"),
];
Expand All @@ -78,15 +78,20 @@ USAGE:
}
args.push(shellEscape(url));

// Use fd 3 to split curl's response body (truncated by `head -c`) from
// the status code written via `-w`. The body goes to stdout via fd 3
// → fd 1, then we append the status code on its own newline.
// Write the body to a temp file rather than a process substitution: `>(…)`
// is a bash-ism that does not expand in the sandbox's exec shell, so curl
// wrote to a file literally named `>(head -c … >&3)`, failed, and exited 23
// with an empty body — which the old exit-23 branch reported as a truncated
// success (chat#1918). stdout contract: `status\nbyteSize\n<body>`, so the
// real size is known even though the body itself is capped for context.
const command = [
"exec 3>&1",
"tmp=$(mktemp)",
`status=$(${args.join(" ")})`,
"curlExit=$?",
"exec 3>&-",
"printf '\\n%s' \"$status\"",
'size=$(wc -c < "$tmp" | tr -d " ")',
'printf \'%s\\n%s\\n\' "$status" "$size"',
`head -c ${MAX_BODY_LENGTH} "$tmp"`,

@coderabbitai coderabbitai Bot Jul 31, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 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.

@cubic-dev-ai cubic-dev-ai Bot Jul 31, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>

'rm -f "$tmp"',
"exit $curlExit",
].join("\n");

Expand All @@ -96,25 +101,34 @@ USAGE:
...(recoupEnv ? { env: recoupEnv } : {}),
});

// exit 23 = curl wrote partial output (`head -c` cut it off — expected for large responses).
if (result.exitCode !== 0 && result.exitCode !== 23) {
const output = result.stdout ?? "";
const firstNewline = output.indexOf("\n");
const secondNewline = output.indexOf("\n", firstNewline + 1);
const statusText = firstNewline === -1 ? "" : output.slice(0, firstNewline).trim();
const sizeText =
secondNewline === -1 ? "" : output.slice(firstNewline + 1, secondNewline).trim();
const responseBody = secondNewline === -1 ? "" : output.slice(secondNewline + 1);
const status = /^\d+$/.test(statusText) && statusText !== "000" ? Number(statusText) : null;
const size = /^\d+$/.test(sizeText) ? Number(sizeText) : responseBody.length;

// 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) {

@cubic-dev-ai cubic-dev-ai Bot Jul 31, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>
Suggested change
if (result.exitCode !== 0 && size === 0) {
if (result.exitCode !== 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"
}`,
};
Comment on lines +117 to 123

@coderabbitai coderabbitai Bot Jul 31, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 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.

Suggested change
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.

}

const output = result.stdout ?? "";
const lastNewline = output.lastIndexOf("\n");
const statusText = lastNewline !== -1 ? output.slice(lastNewline + 1).trim() : "";
const responseBody = lastNewline !== -1 ? output.slice(0, lastNewline) : output;
const status = /^\d+$/.test(statusText) ? parseInt(statusText, 10) : null;

return {
success: true,
status,
body: responseBody,
truncated: result.exitCode === 23,
// Truncation is a property of the response size, not of curl's exit code.
truncated: size > MAX_BODY_LENGTH,
};
} catch (error) {
const message = error instanceof Error ? error.message : String(error);
Expand Down
Loading