Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
11 changes: 6 additions & 5 deletions autogpt_platform/backend/backend/copilot/prompting.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,11 +75,12 @@
}}
```

### Writing large files — CRITICAL
**Never write an entire large document in a single tool call.** When the
content you want to write exceeds ~2000 words the tool call's output token
limit will silently truncate the arguments, producing an empty `{{}}` input
that fails repeatedly.
### Writing large files — CRITICAL (causes production failures)
**NEVER write an entire large document in a single tool call.** When the
content you want to write exceeds ~2000 words the API output-token limit
will silently truncate the tool call arguments mid-JSON, losing all content
and producing an opaque error. This is unrecoverable — the user's work is
lost and retrying with the same approach fails in an infinite loop.

**Preferred: compose from file references.** If the data is already in
files (tool outputs, workspace files), compose the report in one call
Expand Down
20 changes: 19 additions & 1 deletion autogpt_platform/backend/backend/copilot/sdk/e2b_file_tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,11 @@ async def _handle_read_file(args: dict[str, Any]) -> dict[str, Any]:
return _mcp(numbered)


# Inline content above this threshold triggers a warning — it survived this
# time but is dangerously close to the API output-token truncation limit.
_LARGE_CONTENT_WARN_CHARS = 50_000


async def _handle_write_file(args: dict[str, Any]) -> dict[str, Any]:
"""Write content to a sandbox file, creating parent directories as needed."""
file_path: str = args.get("file_path", "")
Expand Down Expand Up @@ -207,7 +212,20 @@ async def _handle_write_file(args: dict[str, Any]) -> dict[str, Any]:
except Exception as exc:
return _mcp(f"Failed to write {remote}: {exc}", error=True)

return _mcp(f"Successfully wrote to {remote}")
msg = f"Successfully wrote to {remote}"
if len(content) > _LARGE_CONTENT_WARN_CHARS:
logger.warning(
"[E2B] write_file: large inline content (%d chars) for %s",
len(content),
remote,
)
msg += (
f"\n\nWARNING: The content was very large ({len(content)} chars). "
"Next time, write large files in sections using bash_exec with "
"'cat > file << EOF ... EOF' and 'cat >> file << EOF ... EOF' "
"to avoid output-token truncation."
)
return _mcp(msg)


async def _handle_edit_file(args: dict[str, Any]) -> dict[str, Any]:
Expand Down
29 changes: 29 additions & 0 deletions autogpt_platform/backend/backend/copilot/sdk/tool_adapter.py
Original file line number Diff line number Diff line change
Expand Up @@ -527,6 +527,28 @@ async def wrapper(args: dict[str, Any]) -> dict[str, Any]:
f"be truncated again."
)

# Partial truncation: the API cut the JSON mid-way — `content` or
# `new_string` survived but `file_path` (emitted later) was lost.
if (
tool_name in ("write_file", "write_workspace_file", "edit_file")
and args
and "file_path" not in args
and ("content" in args or "new_string" in args)
):
logger.warning(
"[MCP] %s: partial truncation detected — file_path missing",
tool_name,
)
return _mcp_error(
f"Your {tool_name} call was truncated (file_path missing). "
"The content was too large for a single tool call. "
"Write in chunks: use bash_exec with "
"'cat > file << EOF ... EOF' for the first section, "
"'cat >> file << EOF ... EOF' to append subsequent "
"sections, then reference the file with "
"@@agptfile:/path/to/file if needed."
)
Comment on lines +530 to +550

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

This guard blocks valid write_workspace_file(content=...) calls.

write_workspace_file does not have a file_path argument, so this condition treats every normal inline-content call as “partial truncation” and short-circuits before the tool runs. It also misses the other valid payload inputs for that tool (content_base64 and source_path).

Proposed fix
-        if (
-            tool_name in ("write_file", "write_workspace_file", "edit_file")
-            and args
-            and "file_path" not in args
-            and ("content" in args or "new_string" in args)
-        ):
+        truncation_checks = {
+            "write_file": ("file_path", {"content"}),
+            "edit_file": ("file_path", {"new_string"}),
+            "write_workspace_file": (
+                "filename",
+                {"content", "content_base64", "source_path"},
+            ),
+        }
+        required_key, payload_keys = truncation_checks.get(tool_name, (None, set()))
+        if (
+            required_key
+            and args
+            and required_key not in args
+            and any(key in args for key in payload_keys)
+        ):
             logger.warning(
-                "[MCP] %s: partial truncation detected — file_path missing",
+                "[MCP] %s: partial truncation detected — %s missing",
                 tool_name,
+                required_key,
             )
             return _mcp_error(
-                f"Your {tool_name} call was truncated (file_path missing). "
+                f"Your {tool_name} call was truncated ({required_key} missing). "
                 "The content was too large for a single tool call. "
                 "Write in chunks: use bash_exec with "
                 "'cat > file << EOF ... EOF' for the first section, "
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@autogpt_platform/backend/backend/copilot/sdk/tool_adapter.py` around lines
530 - 550, The guard treats any call to write_workspace_file with inline content
as truncated because write_workspace_file doesn't use file_path; update the
conditional in the truncation-detection block to either exclude
"write_workspace_file" from the tool_name tuple or to check allowed payload keys
for that tool (e.g., treat write_workspace_file as valid if args contains
"content" or "content_base64" or "source_path"); ensure tools that truly need
file_path (write_file, edit_file) still trigger the _mcp_error when "file_path"
is missing, and keep the existing logger.warning and _mcp_error behavior for
those cases.


original_args = args
stop_msg = _check_circuit_breaker(tool_name, original_args)
if stop_msg:
Expand Down Expand Up @@ -655,10 +677,17 @@ def create_copilot_mcp_server(*, use_e2b: bool = False):
# WebFetch: SSRF risk — can reach internal network (localhost, 10.x, etc.).
# Agent uses the SSRF-protected mcp__copilot__web_fetch tool instead.
# AskUserQuestion: interactive CLI tool — no terminal in copilot context.
# Write: the CLI's built-in Write tool has no defence against output-token
# truncation. When the LLM generates a very large `content` argument the
# API truncates the response mid-JSON and Ajv rejects it with the opaque
# "'file_path' is a required property" error, losing the user's work.
# All writes go through our MCP write_file / write_workspace_file tools
# where we control validation and return actionable guidance.
SDK_DISALLOWED_TOOLS = [
"Bash",
"WebFetch",
"AskUserQuestion",
"Write",
]

# Tools that are blocked entirely in security hooks (defence-in-depth).
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -929,3 +929,101 @@ async def fake_tool_fn(_args: dict) -> dict:
stashed = pop_pending_tool_output("fake_tool_normal")
assert stashed is not None
assert '"is_dry_run": true' in stashed


# ---------------------------------------------------------------------------
# Partial truncation detection (Layer 2 of write-tool-truncation fix)
# ---------------------------------------------------------------------------


class TestPartialTruncationDetection:
"""When the API truncates a write tool call mid-JSON, `content` may survive
but `file_path` is lost. The wrapper must detect this and return actionable
guidance instead of letting the call through to fail with an opaque error.
"""

@pytest.mark.asyncio
async def test_write_file_partial_truncation_detected(self):
"""write_file with content but no file_path returns truncation error."""

async def fake_tool_fn(_args: dict) -> dict:
raise AssertionError("Should not be called")

wrapper = _make_truncating_wrapper(
fake_tool_fn,
"write_file",
input_schema={"required": ["file_path", "content"]},
)
result = await wrapper({"content": "some data"})
assert result["isError"] is True
text = result["content"][0]["text"]
assert "truncated" in text
assert "file_path" in text

@pytest.mark.asyncio
async def test_edit_file_partial_truncation_detected(self):
"""edit_file with new_string but no file_path returns truncation error."""

async def fake_tool_fn(_args: dict) -> dict:
raise AssertionError("Should not be called")

wrapper = _make_truncating_wrapper(
fake_tool_fn,
"edit_file",
input_schema={"required": ["file_path", "old_string", "new_string"]},
)
result = await wrapper({"new_string": "replacement"})
assert result["isError"] is True
text = result["content"][0]["text"]
assert "truncated" in text

@pytest.mark.asyncio
async def test_write_file_with_file_path_passes_through(self):
"""write_file with file_path present should NOT trigger truncation guard."""
called = False

async def fake_tool_fn(_args: dict) -> dict:
nonlocal called
called = True
return {
"content": [{"type": "text", "text": "ok"}],
"isError": False,
}

normal_session = MagicMock()
normal_session.dry_run = False
set_execution_context(user_id="test", session=normal_session, sandbox=None, sdk_cwd="/tmp/test") # type: ignore[arg-type]

wrapper = _make_truncating_wrapper(
fake_tool_fn,
"write_file",
input_schema={"required": ["file_path", "content"]},
)
result = await wrapper({"file_path": "/home/user/f.txt", "content": "data"})
assert called
assert result["isError"] is False

@pytest.mark.asyncio
async def test_non_write_tool_not_affected(self):
"""Partial truncation guard only applies to write/edit tools."""
called = False

async def fake_tool_fn(_args: dict) -> dict:
nonlocal called
called = True
return {
"content": [{"type": "text", "text": "ok"}],
"isError": False,
}

normal_session = MagicMock()
normal_session.dry_run = False
set_execution_context(user_id="test", session=normal_session, sandbox=None, sdk_cwd="/tmp/test") # type: ignore[arg-type]

wrapper = _make_truncating_wrapper(
fake_tool_fn,
"bash_exec",
input_schema={"required": ["command"]},
)
await wrapper({"content": "some data"})
assert called
Comment on lines +939 to +1029

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

Please add a write_workspace_file case to this suite.

The new wrapper logic also branches on write_workspace_file, but this class only exercises write_file and edit_file. A write_workspace_file(content=...) regression test would have caught the current file_path/filename mix-up immediately.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@autogpt_platform/backend/backend/copilot/sdk/tool_adapter_test.py` around
lines 939 - 1029, Add a new test in TestPartialTruncationDetection that mirrors
test_write_file_partial_truncation_detected but uses the "write_workspace_file"
tool name so the truncation guard for workspace writes is exercised; use
_make_truncating_wrapper(fake_tool_fn, "write_workspace_file",
input_schema={"required": ["filename", "content"]}) and call wrapper({"content":
"some data"}) asserting result["isError"] is True and the returned text mentions
"truncated" and the missing "filename"/"file_path" as appropriate; also add a
positive case like test_write_file_with_file_path_passes_through but for
write_workspace_file to ensure calls with "filename" pass through and the fake
tool is invoked.

Original file line number Diff line number Diff line change
Expand Up @@ -800,14 +800,18 @@ async def _execute(
if not has_any_content:
return ErrorResponse(
message=(
"Tool call appears truncated (no arguments received). "
"This happens when the content is too large for a "
"single tool call. Instead of passing content inline, "
"first write the file to the working directory using "
"bash_exec (e.g. cat > /home/user/file.md << 'EOF'... "
"EOF), then use source_path to copy it to workspace: "
"write_workspace_file(filename='file.md', "
"source_path='/home/user/file.md')"
"Your file write was truncated because the content "
"was too large for a single tool call (all arguments "
"were lost). Instead of passing content inline, write "
"the file in sections:\n"
'1. Use bash_exec with \'cat > filename << "EOF"\\n'
"...\\nEOF' for the first section\n"
"2. Use 'cat >> filename << \"EOF\"\\n...\\nEOF' to "
"append more sections\n"
"3. Then use write_workspace_file(filename=..., "
"source_path=...) to save it to workspace\n"
"Do NOT retry with the same approach — it will be "
"truncated again."
),
session_id=session_id,
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -623,3 +623,46 @@ class _FakeFileInfo:
# Normal workspace path must have produced a content response.
assert isinstance(result, WorkspaceFileContentResponse)
assert base64.b64decode(result.content_base64) == fake_content


# ---------------------------------------------------------------------------
# Truncation error message (Layer 4 of write-tool-truncation fix)
# ---------------------------------------------------------------------------


class TestWriteTruncationMessage:
"""When write_workspace_file receives empty args due to API output-token
truncation, the error message must give actionable step-by-step guidance."""

@pytest.mark.asyncio
async def test_empty_args_returns_actionable_truncation_message(
self, ephemeral_dir
):
"""Calling write with no args at all should mention cat > and cat >>."""
write_tool = WriteWorkspaceFileTool()
result = await write_tool._execute(
user_id="test-user",
session=make_session(),
filename=None, # type: ignore[arg-type]
)
assert isinstance(result, ErrorResponse)
assert "truncated" in result.message.lower()
assert "cat >" in result.message
assert "cat >>" in result.message
assert "source_path" in result.message

@pytest.mark.asyncio
async def test_missing_filename_with_content_gives_simple_error(
self, ephemeral_dir
):
"""When content is present but filename is missing, it's not truncation —
just a missing required field."""
write_tool = WriteWorkspaceFileTool()
result = await write_tool._execute(
user_id="test-user",
session=make_session(),
filename=None, # type: ignore[arg-type]
content="some content",
)
Comment on lines +638 to +666

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

Remove the new # type: ignore suppressors here.

These tests add two # type: ignore[arg-type] escapes for filename=None. Please model this path in the types instead — e.g. let _execute() accept filename: str | None for malformed/truncated calls, or route the call through a typed helper — so the tests can stay suppression-free.

As per coding guidelines, "Do not use linter suppressors — no # type: ignore, # noqa, # pyright: ignore; fix the type/code instead".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@autogpt_platform/backend/backend/copilot/tools/workspace_files_test.py`
around lines 638 - 666, The tests currently use "# type: ignore[arg-type]" for
filename=None; instead update the implementation rather than suppressing types:
change WriteWorkspaceFileTool._execute (and any interface it implements) to
accept filename: str | None (or overloads/typed helper e.g.,
_execute_with_optional_filename) and handle the None case (truncated/malformed
vs missing required field) internally, then remove the type ignores from tests;
ensure any callers or type stubs of WriteWorkspaceFileTool._execute are updated
to the new signature so mypy/pyright no longer requires suppressors.

assert isinstance(result, ErrorResponse)
assert "filename" in result.message.lower()
Loading