fix(copilot): unified MCP file tools (Read/Write/Edit) to prevent truncation data loss - #12750
Merged
Conversation
Replace the CLI's built-in Write tool with a unified MCP Write tool that detects and handles output-token truncation gracefully instead of losing user work with opaque "'file_path' is a required property" errors. The new tool: - Works in both E2B and non-E2B modes - Detects partial truncation (content present but file_path missing) - Detects complete truncation (empty args) with actionable guidance - Warns on large content (>50K chars) that succeeded - Places file_path first in schema to survive truncation better - Validates paths stay within the SDK working directory Also blocks the CLI built-in Write via SDK_DISALLOWED_TOOLS and strengthens the system prompt's large-file writing guidance. Fixes production truncation bug reported in sessions: 1a0ef47f-9711-41d2-91b8-df9dff9ecfc6 2bb9fb0d-05bd-4194-8f3f-88fbe3d8b965 8226d82e-bf9c-48e3-826f-80048d0fb7b4
majdyz
requested review from
0ubbe and
Bentlybro
and removed request for
a team
April 11, 2026 17:17
github-project-automation
Bot
moved this to 🆕 Needs initial review
in AutoGPT development kanban
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds unified MCP file tools (Write, Read, Edit) with truncation detection, path resolution/validation, E2B delegation, MCP registration and gating; adds comprehensive non‑E2B tests; updates copilot prompting guidance to clarify ~2000‑word API output‑token truncation behavior. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant MCP as MCP_Server
participant Handler as File_Tool_Handler
participant FS as File_System
participant E2B as E2B_Sandbox
Client->>MCP: mcp__copilot__Write/Read/Edit(args)
MCP->>Handler: dispatch (use_e2b flag)
Handler->>Handler: _check_truncation(args)
alt non-E2B
Handler->>Handler: resolve & validate path (get_sdk_cwd / is_allowed_local_path)
Handler->>FS: read/write/create parent dirs as needed
FS-->>Handler: file contents / write result
Handler-->>MCP: structured success or MCP error
else E2B
Handler->>E2B: delegate operation
E2B->>FS: sandboxed file op
FS-->>E2B: result
E2B-->>Handler: result
Handler-->>MCP: structured success or MCP error
end
MCP-->>Client: return MCP response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
Contributor
🔍 PR Overlap DetectionThis check compares your PR against all other open PRs targeting the same branch to detect potential merge conflicts early. 🔴 Merge Conflicts DetectedThe following PRs have been tested and will have merge conflicts if merged after this PR. Consider coordinating with the authors.
🟡 Medium Risk — Some Line OverlapThese PRs have some overlapping changes:
🟢 Low Risk — File Overlap OnlyThese PRs touch the same files but different sections (click to expand)
Summary: 1 conflict(s), 1 medium risk, 19 low risk (out of 21 PRs with file overlap) Auto-generated on push. Ignores: |
…e error paths - DRY: extract _check_truncation() shared between E2B and non-E2B handlers - Security: use os.path.basename() in path validation errors to avoid leaking internal directory structure
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@autogpt_platform/backend/backend/copilot/sdk/file_tools.py`:
- Around line 103-104: The exception handler currently returns the full absolute
path via the variable resolved in the message (calling _mcp(f"Failed to write
{resolved}: {exc}", ...)), which can leak internal directory structure; update
the handler in file_tools.py to sanitize the path by using
os.path.basename(resolved) (or a similar safe name) in the message and keep the
original exception for logging if needed, e.g., build the user-facing error
string with basename_resolved = os.path.basename(resolved) and call
_mcp(f"Failed to write {basename_resolved}: {exc}", error=True); ensure you
import os if not already present and do not expose the full resolved value in
the returned message.
- Around line 90-102: The write path currently does a check-then-act
(is_allowed_local_path(resolved, sdk_cwd)) allowing a TOCTOU symlink swap; fix
by avoiding check-then-open: after the initial path validation, open the target
using low-level os.open with flags that prevent following symlinks (e.g.,
include O_NOFOLLOW) and write via os.fdopen to atomically obtain an FD you
control, then re-verify the opened FD points inside sdk_cwd (for example resolve
/proc/self/fd/{fd} or use fstat + realpath) before writing; use
O_CREAT|O_TRUNC|O_WRONLY (and O_EXCL if you must disallow replacing existing
files) when calling os.open and ensure parents are created safely beforehand
(os.makedirs for parent) while referencing is_allowed_local_path, resolved,
sdk_cwd, and the write block that currently opens the file.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0ab78cbf-6063-4d4f-879c-c483d489b981
📒 Files selected for processing (4)
autogpt_platform/backend/backend/copilot/prompting.pyautogpt_platform/backend/backend/copilot/sdk/file_tools.pyautogpt_platform/backend/backend/copilot/sdk/file_tools_test.pyautogpt_platform/backend/backend/copilot/sdk/tool_adapter.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: check API types
- GitHub Check: end-to-end tests
- GitHub Check: test (3.11)
- GitHub Check: test (3.12)
- GitHub Check: test (3.13)
- GitHub Check: Seer Code Review
- GitHub Check: Analyze (python)
- GitHub Check: Check PR Status
🧰 Additional context used
📓 Path-based instructions (3)
autogpt_platform/backend/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
autogpt_platform/backend/**/*.py: Use Python 3.11 (required; managed by Poetry via pyproject.toml) for backend development
Always run 'poetry run format' (Black + isort) before linting in backend development
Always run 'poetry run lint' (ruff) after formatting in backend development
autogpt_platform/backend/**/*.py: Usepoetry run ...command for executing Python package dependencies
Use top-level imports only — avoid local/inner imports except for lazy imports of heavy optional dependencies likeopenpyxl
Use absolute imports withfrom backend.module import ...for cross-package imports; single-dot relative imports are acceptable for sibling modules within the same package; avoid double-dot relative imports
Do not use duck typing — avoidhasattr/getattr/isinstancefor type dispatch; use typed interfaces/unions/protocols instead
Use Pydantic models over dataclass/namedtuple/dict for structured data
Do not use linter suppressors — no# type: ignore,# noqa,# pyright: ignore; fix the type/code instead
Prefer list comprehensions over manual loop-and-append patterns
Use early return with guard clauses first to avoid deep nesting
Use%sfor deferred interpolation indebuglog statements for efficiency; use f-strings elsewhere for readability (e.g.,logger.debug("Processing %s items", count)vslogger.info(f"Processing {count} items"))
Sanitize error paths by usingos.path.basename()in error messages to avoid leaking directory structure
Be aware of TOCTOU (Time-Of-Check-Time-Of-Use) issues — avoid check-then-act patterns for file access and credit charging
Usetransaction=Truefor Redis pipelines to ensure atomicity on multi-step operations
Usemax(0, value)guards for computed values that should never be negative
Keep files under ~300 lines; if a file grows beyond this, split by responsibility (extract helpers, models, or a sub-module into a new file)
Keep functions under ~40 lines; extract named helpers when a function grows longer
...
Files:
autogpt_platform/backend/backend/copilot/prompting.pyautogpt_platform/backend/backend/copilot/sdk/tool_adapter.pyautogpt_platform/backend/backend/copilot/sdk/file_tools.pyautogpt_platform/backend/backend/copilot/sdk/file_tools_test.py
autogpt_platform/{backend,autogpt_libs}/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Format Python code with
poetry run format
Files:
autogpt_platform/backend/backend/copilot/prompting.pyautogpt_platform/backend/backend/copilot/sdk/tool_adapter.pyautogpt_platform/backend/backend/copilot/sdk/file_tools.pyautogpt_platform/backend/backend/copilot/sdk/file_tools_test.py
autogpt_platform/backend/**/*_test.py
📄 CodeRabbit inference engine (autogpt_platform/backend/AGENTS.md)
autogpt_platform/backend/**/*_test.py: Use pytest with snapshot testing for API responses
Colocate test files with source files using*_test.pynaming convention
Mock at boundaries — mock where the symbol is used, not where it's defined; after refactoring, update mock targets to match new module paths
UseAsyncMockfromunittest.mockfor async functions in tests
When writing tests, use Test-Driven Development (TDD): write failing tests marked with@pytest.mark.xfailbefore implementation, then remove the marker once the implementation is complete
When creating snapshots in tests, usepoetry run pytest path/to/test.py --snapshot-update; always review snapshot changes withgit diffbefore committing
Files:
autogpt_platform/backend/backend/copilot/sdk/file_tools_test.py
🧠 Learnings (24)
📓 Common learnings
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12632
File: autogpt_platform/backend/backend/copilot/tools/workspace_files.py:0-0
Timestamp: 2026-04-01T04:17:43.495Z
Learning: In autogpt_platform/backend/backend/copilot/sdk/tool_adapter.py (PR `#12632`, commit 12ae03c), the per-tool `BaseTool.read_only` property approach was removed. Instead, `readOnlyHint=True` (via `ToolAnnotations`) is applied unconditionally to ALL tools — including side-effect tools like `bash_exec` and `write_workspace_file` — to enable fully parallel dispatch by the Anthropic SDK/CLI. Do not flag tools with mutating operations (e.g. save_to_path, write operations) for having `readOnlyHint=True`; this is intentional and E2E validated (3x bash_exec(sleep 3) completed in 3.3s vs 9s sequential).
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12213
File: autogpt_platform/frontend/src/app/api/openapi.json:9983-9995
Timestamp: 2026-02-27T15:59:00.370Z
Learning: Repo: Significant-Gravitas/AutoGPT PR: 12213 — OpenAPI/codegen
Learning: Ensuring a field is required in generated TS types needs two sides: (1) no default value on the Pydantic field, and (2) the OpenAPI model's "required" array must list it. For MCPToolInfo, making input_schema required in OpenAPI and removing Field(default_factory=dict) in the backend prevents optional typing drift.
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12213
File: autogpt_platform/frontend/src/app/api/openapi.json:9983-9995
Timestamp: 2026-02-27T15:59:00.370Z
Learning: Repo: Significant-Gravitas/AutoGPT PR: 12213 — Backend/frontend OpenAPI codegen
Learning: For MCP schema models, required OpenAPI fields must have no defaults in Pydantic. Specifically, MCPToolInfo.input_schema must be required (no Field(default_factory=dict)) so openapi.json emits it in "required", ensuring generated TS types treat input_schema as non-optional.
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12213
File: autogpt_platform/frontend/src/app/api/openapi.json:10030-10037
Timestamp: 2026-03-01T07:59:02.311Z
Learning: Repo: Significant-Gravitas/AutoGPT PR: 12213 — For MCP manual token storage, backend model autogpt_platform/backend/backend/api/features/mcp/routes.py defines MCPStoreTokenRequest.token as Pydantic SecretStr with a min length constraint, which generates OpenAPI schema metadata (format: "password", writeOnly: true, minLength: 1) in autogpt_platform/frontend/src/app/api/openapi.json. Prefer SecretStr (with length constraints) for sensitive request fields so generated TS clients and docs treat them as secrets.
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12632
File: autogpt_platform/backend/backend/copilot/sdk/tool_adapter.py:530-535
Timestamp: 2026-04-01T04:17:38.279Z
Learning: In `autogpt_platform/backend/backend/copilot/sdk/tool_adapter.py`, the `ToolAnnotations(readOnlyHint=True)` annotation (stored as `_PARALLEL_ANNOTATION`) is intentionally applied to ALL registered MCP tools — including E2B write/edit tools (e.g., `write_file`, `edit_file`). This is a parallel-dispatch hint to the Claude Agent SDK CLI, not a semantic read-only contract. The `_READ_ONLY_E2B_TOOLS` set was dead code and was removed in commit `12ae03c`; the constant was renamed from `_READONLY_ANNOTATION` to `_PARALLEL_ANNOTATION` in commit `c88ca88` to avoid confusion. Do not flag this as a correctness issue.
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12440
File: autogpt_platform/backend/backend/copilot/workflow_import/converter.py:0-0
Timestamp: 2026-03-17T10:57:12.953Z
Learning: In Significant-Gravitas/AutoGPT PR `#12440`, `autogpt_platform/backend/backend/copilot/workflow_import/converter.py` was fully rewritten (commit 732960e2d) to no longer make direct LLM/OpenAI API calls. The converter now builds a structured text prompt for AutoPilot/CoPilot instead. There is no `response.choices` access or any direct LLM client usage in this file. Do not flag `response.choices` access or LLM client initialization patterns as issues in this file.
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12213
File: autogpt_platform/frontend/src/app/(platform)/copilot/tools/RunMCPTool/helpers.tsx:23-24
Timestamp: 2026-02-27T10:45:55.700Z
Learning: As of PR `#12213`, MCP tool response types (MCPToolsDiscoveredResponse, MCPToolOutputResponse) are defined in openapi.json and frontend code in autogpt_platform/frontend/src/app/(platform)/copilot/tools/RunMCPTool/helpers.tsx uses the generated types from `@/app/api/__generated__/`. Other tools like RunBlock still use inline TypeScript interfaces (e.g., BlockDetailsResponse) for SSE stream payloads that are not included in openapi.json schemas. The pattern is tool-specific: use generated types when available in openapi.json, use inline types only when the payload schema is truly SSE-stream-only and not exposed via OpenAPI.
📚 Learning: 2026-03-17T10:57:12.953Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12440
File: autogpt_platform/backend/backend/copilot/workflow_import/converter.py:0-0
Timestamp: 2026-03-17T10:57:12.953Z
Learning: In Significant-Gravitas/AutoGPT PR `#12440`, `autogpt_platform/backend/backend/copilot/workflow_import/converter.py` was fully rewritten (commit 732960e2d) to no longer make direct LLM/OpenAI API calls. The converter now builds a structured text prompt for AutoPilot/CoPilot instead. There is no `response.choices` access or any direct LLM client usage in this file. Do not flag `response.choices` access or LLM client initialization patterns as issues in this file.
Applied to files:
autogpt_platform/backend/backend/copilot/prompting.pyautogpt_platform/backend/backend/copilot/sdk/tool_adapter.pyautogpt_platform/backend/backend/copilot/sdk/file_tools.py
📚 Learning: 2026-03-09T10:50:43.907Z
Learnt from: Bentlybro
Repo: Significant-Gravitas/AutoGPT PR: 0
File: :0-0
Timestamp: 2026-03-09T10:50:43.907Z
Learning: Repo: Significant-Gravitas/AutoGPT — File: autogpt_platform/backend/backend/blocks/llm.py
For xAI Grok models accessed via OpenRouter, the API returns `null` for `max_completion_tokens`. The convention in this codebase is to use the model's context window size as the `max_output_tokens` value in ModelMetadata. For example, Grok 3 uses 131072 (128k) and Grok 4 uses 262144 (256k). Do not flag these as incorrect max output token values.
Applied to files:
autogpt_platform/backend/backend/copilot/prompting.py
📚 Learning: 2026-03-01T07:59:02.311Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12213
File: autogpt_platform/frontend/src/app/api/openapi.json:10030-10037
Timestamp: 2026-03-01T07:59:02.311Z
Learning: Repo: Significant-Gravitas/AutoGPT PR: 12213 — For MCP manual token storage, backend model autogpt_platform/backend/backend/api/features/mcp/routes.py defines MCPStoreTokenRequest.token as Pydantic SecretStr with a min length constraint, which generates OpenAPI schema metadata (format: "password", writeOnly: true, minLength: 1) in autogpt_platform/frontend/src/app/api/openapi.json. Prefer SecretStr (with length constraints) for sensitive request fields so generated TS clients and docs treat them as secrets.
Applied to files:
autogpt_platform/backend/backend/copilot/prompting.py
📚 Learning: 2026-04-08T17:28:23.422Z
Learnt from: CR
Repo: Significant-Gravitas/AutoGPT PR: 0
File: autogpt_platform/backend/AGENTS.md:0-0
Timestamp: 2026-04-08T17:28:23.422Z
Learning: Applies to autogpt_platform/backend/**/*.py : Keep files under ~300 lines; if a file grows beyond this, split by responsibility (extract helpers, models, or a sub-module into a new file)
Applied to files:
autogpt_platform/backend/backend/copilot/prompting.py
📚 Learning: 2026-03-24T21:25:15.983Z
Learnt from: ntindle
Repo: Significant-Gravitas/AutoGPT PR: 12536
File: autogpt_platform/frontend/src/app/api/openapi.json:5770-5790
Timestamp: 2026-03-24T21:25:15.983Z
Learning: Repo: Significant-Gravitas/AutoGPT — PR `#12536`
File: autogpt_platform/frontend/src/app/api/openapi.json
Learning: The OpenAPI spec file is auto-generated; per established convention, endpoints generally declare only 200/201, 401, and 422 responses. Do not suggest adding explicit 403/404 response entries for single operations unless planning a repo-wide spec update. Prefer clarifying such behaviors in endpoint descriptions/docstrings instead of altering response maps.
Applied to files:
autogpt_platform/backend/backend/copilot/prompting.py
📚 Learning: 2026-04-01T04:17:41.600Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12632
File: autogpt_platform/backend/backend/copilot/tools/workspace_files.py:0-0
Timestamp: 2026-04-01T04:17:41.600Z
Learning: When reviewing AutoGPT Copilot tool implementations, accept that `readOnlyHint=True` (provided via `ToolAnnotations`) may be applied unconditionally to *all* tools—even tools that have side effects (e.g., `bash_exec`, `write_workspace_file`, or other write/save operations). Do **not** flag these tools for having `readOnlyHint=True`; this is intentional to enable fully-parallel dispatch by the Anthropic SDK/CLI and has been E2E validated. Only flag `readOnlyHint` issues if they conflict with the established `ToolAnnotations` behavior (e.g., missing/incorrect propagation relative to the intended annotation mechanism).
Applied to files:
autogpt_platform/backend/backend/copilot/prompting.pyautogpt_platform/backend/backend/copilot/sdk/tool_adapter.pyautogpt_platform/backend/backend/copilot/sdk/file_tools.pyautogpt_platform/backend/backend/copilot/sdk/file_tools_test.py
📚 Learning: 2026-03-17T06:48:26.471Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12445
File: autogpt_platform/backend/backend/copilot/sdk/service.py:1071-1072
Timestamp: 2026-03-17T06:48:26.471Z
Learning: In Significant-Gravitas/AutoGPT (autogpt_platform), the AI SDK enforces `z.strictObject({type, errorText})` on SSE `StreamError` responses, so additional fields like `retryable: bool` cannot be added to `StreamError` or serialized via `to_sse()`. Instead, retry signaling for transient Anthropic API errors is done via the `COPILOT_RETRYABLE_ERROR_PREFIX` constant prepended to persisted session messages (in `ChatMessage.content`). The frontend detects retryable errors by checking `markerType === "retryable_error"` from `parseSpecialMarkers()` — no SSE schema changes and no string matching on error text. This pattern was established in PR `#12445`, commit 64d82797b.
Applied to files:
autogpt_platform/backend/backend/copilot/prompting.py
📚 Learning: 2026-04-03T11:14:16.378Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12623
File: autogpt_platform/backend/backend/copilot/transcript_builder.py:30-34
Timestamp: 2026-04-03T11:14:16.378Z
Learning: In `autogpt_platform/backend/backend/copilot/transcript_builder.py` (and its re-export shim at `sdk/transcript_builder.py`), `TranscriptEntry.parentUuid` is typed `str` (not `str | None`) and root entries use `parentUuid=""` (empty string) to match the canonical `_messages_to_transcript` JSONL format. `_parse_entry`, `append_user`, and `append_assistant` all coerce `None` to `""`. Do NOT flag `parentUuid=""` as incorrect — it is the correct root marker. This was fixed in PR `#12623`, commit b753cb7d0b.
Applied to files:
autogpt_platform/backend/backend/copilot/prompting.py
📚 Learning: 2026-02-26T17:02:22.448Z
Learnt from: Pwuts
Repo: Significant-Gravitas/AutoGPT PR: 12211
File: .pre-commit-config.yaml:160-179
Timestamp: 2026-02-26T17:02:22.448Z
Learning: Keep the pre-commit hook pattern broad for autogpt_platform/backend to ensure OpenAPI schema changes are captured. Do not narrow to backend/api/ alone, since the generated schema depends on Pydantic models across multiple directories (backend/data/, backend/blocks/, backend/copilot/, backend/integrations/, backend/util/). Narrowing could miss schema changes and cause frontend type desynchronization.
Applied to files:
autogpt_platform/backend/backend/copilot/prompting.pyautogpt_platform/backend/backend/copilot/sdk/tool_adapter.pyautogpt_platform/backend/backend/copilot/sdk/file_tools.pyautogpt_platform/backend/backend/copilot/sdk/file_tools_test.py
📚 Learning: 2026-03-04T08:04:35.881Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12273
File: autogpt_platform/backend/backend/copilot/tools/workspace_files.py:216-220
Timestamp: 2026-03-04T08:04:35.881Z
Learning: In the AutoGPT Copilot backend, ensure that SVG images are not treated as vision image types by excluding 'image/svg+xml' from INLINEABLE_MIME_TYPES and MULTIMODAL_TYPES in tool_adapter.py; the Claude API supports PNG, JPEG, GIF, and WebP for vision. SVGs (XML text) should be handled via the text path instead, not the vision path.
Applied to files:
autogpt_platform/backend/backend/copilot/prompting.pyautogpt_platform/backend/backend/copilot/sdk/tool_adapter.pyautogpt_platform/backend/backend/copilot/sdk/file_tools.pyautogpt_platform/backend/backend/copilot/sdk/file_tools_test.py
📚 Learning: 2026-03-05T15:42:08.207Z
Learnt from: ntindle
Repo: Significant-Gravitas/AutoGPT PR: 12297
File: .claude/skills/backend-check/SKILL.md:14-16
Timestamp: 2026-03-05T15:42:08.207Z
Learning: In Python files under autogpt_platform/backend (recursively), rely on poetry run format to perform formatting (Black + isort) and linting (ruff). Do not run poetry run lint as a separate step after poetry run format, since format already includes linting checks.
Applied to files:
autogpt_platform/backend/backend/copilot/prompting.pyautogpt_platform/backend/backend/copilot/sdk/tool_adapter.pyautogpt_platform/backend/backend/copilot/sdk/file_tools.pyautogpt_platform/backend/backend/copilot/sdk/file_tools_test.py
📚 Learning: 2026-03-16T16:35:40.236Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12440
File: autogpt_platform/backend/backend/api/features/workflow_import.py:54-63
Timestamp: 2026-03-16T16:35:40.236Z
Learning: Avoid using the word 'competitor' in public-facing identifiers and text. Use neutral naming for API paths, model names, function names, and UI text. Examples: rename 'CompetitorFormat' to 'SourcePlatform', 'convert_competitor_workflow' to 'convert_workflow', '/competitor-workflow' to '/workflow'. Apply this guideline to files under autogpt_platform/backend and autogpt_platform/frontend.
Applied to files:
autogpt_platform/backend/backend/copilot/prompting.pyautogpt_platform/backend/backend/copilot/sdk/tool_adapter.pyautogpt_platform/backend/backend/copilot/sdk/file_tools.pyautogpt_platform/backend/backend/copilot/sdk/file_tools_test.py
📚 Learning: 2026-03-31T15:37:38.626Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12623
File: autogpt_platform/backend/backend/copilot/tools/agent_generator/fixer.py:37-47
Timestamp: 2026-03-31T15:37:38.626Z
Learning: When validating/constructing Anthropic API model IDs in Significant-Gravitas/AutoGPT, allow the hyphen-separated Claude Opus 4.6 model ID `claude-opus-4-6` (it corresponds to `LlmModel.CLAUDE_4_6_OPUS` in `autogpt_platform/backend/backend/blocks/llm.py`). Do NOT require the dot-separated form in Anthropic contexts. Only OpenRouter routing variants should use the dot separator (e.g., `anthropic/claude-opus-4.6`); `claude-opus-4-6` should be treated as correct when passed to Anthropic, and flagged only if it’s used in the OpenRouter path where the dot form is expected.
Applied to files:
autogpt_platform/backend/backend/copilot/prompting.pyautogpt_platform/backend/backend/copilot/sdk/tool_adapter.pyautogpt_platform/backend/backend/copilot/sdk/file_tools.pyautogpt_platform/backend/backend/copilot/sdk/file_tools_test.py
📚 Learning: 2026-04-01T04:17:38.279Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12632
File: autogpt_platform/backend/backend/copilot/sdk/tool_adapter.py:530-535
Timestamp: 2026-04-01T04:17:38.279Z
Learning: In `autogpt_platform/backend/backend/copilot/sdk/tool_adapter.py`, the `ToolAnnotations(readOnlyHint=True)` annotation (stored as `_PARALLEL_ANNOTATION`) is intentionally applied to ALL registered MCP tools — including E2B write/edit tools (e.g., `write_file`, `edit_file`). This is a parallel-dispatch hint to the Claude Agent SDK CLI, not a semantic read-only contract. The `_READ_ONLY_E2B_TOOLS` set was dead code and was removed in commit `12ae03c`; the constant was renamed from `_READONLY_ANNOTATION` to `_PARALLEL_ANNOTATION` in commit `c88ca88` to avoid confusion. Do not flag this as a correctness issue.
Applied to files:
autogpt_platform/backend/backend/copilot/sdk/tool_adapter.pyautogpt_platform/backend/backend/copilot/sdk/file_tools.pyautogpt_platform/backend/backend/copilot/sdk/file_tools_test.py
📚 Learning: 2026-02-27T10:45:55.700Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12213
File: autogpt_platform/frontend/src/app/(platform)/copilot/tools/RunMCPTool/helpers.tsx:23-24
Timestamp: 2026-02-27T10:45:55.700Z
Learning: As of PR `#12213`, MCP tool response types (MCPToolsDiscoveredResponse, MCPToolOutputResponse) are defined in openapi.json and frontend code in autogpt_platform/frontend/src/app/(platform)/copilot/tools/RunMCPTool/helpers.tsx uses the generated types from `@/app/api/__generated__/`. Other tools like RunBlock still use inline TypeScript interfaces (e.g., BlockDetailsResponse) for SSE stream payloads that are not included in openapi.json schemas. The pattern is tool-specific: use generated types when available in openapi.json, use inline types only when the payload schema is truly SSE-stream-only and not exposed via OpenAPI.
Applied to files:
autogpt_platform/backend/backend/copilot/sdk/tool_adapter.pyautogpt_platform/backend/backend/copilot/sdk/file_tools.py
📚 Learning: 2026-02-27T15:59:00.370Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12213
File: autogpt_platform/frontend/src/app/api/openapi.json:9983-9995
Timestamp: 2026-02-27T15:59:00.370Z
Learning: Repo: Significant-Gravitas/AutoGPT PR: 12213 — OpenAPI/codegen
Learning: Ensuring a field is required in generated TS types needs two sides: (1) no default value on the Pydantic field, and (2) the OpenAPI model's "required" array must list it. For MCPToolInfo, making input_schema required in OpenAPI and removing Field(default_factory=dict) in the backend prevents optional typing drift.
Applied to files:
autogpt_platform/backend/backend/copilot/sdk/tool_adapter.pyautogpt_platform/backend/backend/copilot/sdk/file_tools.pyautogpt_platform/backend/backend/copilot/sdk/file_tools_test.py
📚 Learning: 2026-02-27T15:59:00.370Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12213
File: autogpt_platform/frontend/src/app/api/openapi.json:9983-9995
Timestamp: 2026-02-27T15:59:00.370Z
Learning: Repo: Significant-Gravitas/AutoGPT PR: 12213 — Backend/frontend OpenAPI codegen
Learning: For MCP schema models, required OpenAPI fields must have no defaults in Pydantic. Specifically, MCPToolInfo.input_schema must be required (no Field(default_factory=dict)) so openapi.json emits it in "required", ensuring generated TS types treat input_schema as non-optional.
Applied to files:
autogpt_platform/backend/backend/copilot/sdk/tool_adapter.pyautogpt_platform/backend/backend/copilot/sdk/file_tools.pyautogpt_platform/backend/backend/copilot/sdk/file_tools_test.py
📚 Learning: 2026-04-08T17:28:23.422Z
Learnt from: CR
Repo: Significant-Gravitas/AutoGPT PR: 0
File: autogpt_platform/backend/AGENTS.md:0-0
Timestamp: 2026-04-08T17:28:23.422Z
Learning: Applies to autogpt_platform/backend/**/*_test.py : Colocate test files with source files using `*_test.py` naming convention
Applied to files:
autogpt_platform/backend/backend/copilot/sdk/file_tools_test.py
📚 Learning: 2026-04-08T17:28:23.422Z
Learnt from: CR
Repo: Significant-Gravitas/AutoGPT PR: 0
File: autogpt_platform/backend/AGENTS.md:0-0
Timestamp: 2026-04-08T17:28:23.422Z
Learning: Applies to autogpt_platform/backend/**/*_test.py : When creating snapshots in tests, use `poetry run pytest path/to/test.py --snapshot-update`; always review snapshot changes with `git diff` before committing
Applied to files:
autogpt_platform/backend/backend/copilot/sdk/file_tools_test.py
📚 Learning: 2026-02-04T16:49:42.490Z
Learnt from: CR
Repo: Significant-Gravitas/AutoGPT PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-02-04T16:49:42.490Z
Learning: Applies to autogpt_platform/backend/**/test/**/*.py : Use snapshot testing with '--snapshot-update' flag in backend tests when output changes; always review with 'git diff'
Applied to files:
autogpt_platform/backend/backend/copilot/sdk/file_tools_test.py
📚 Learning: 2026-02-04T16:49:42.490Z
Learnt from: CR
Repo: Significant-Gravitas/AutoGPT PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-02-04T16:49:42.490Z
Learning: Applies to autogpt_platform/backend/backend/blocks/**/*.py : Write tests alongside block implementation when adding new blocks in backend
Applied to files:
autogpt_platform/backend/backend/copilot/sdk/file_tools_test.py
📚 Learning: 2026-04-08T17:28:23.422Z
Learnt from: CR
Repo: Significant-Gravitas/AutoGPT PR: 0
File: autogpt_platform/backend/AGENTS.md:0-0
Timestamp: 2026-04-08T17:28:23.422Z
Learning: Applies to autogpt_platform/backend/**/*_test.py : When writing tests, use Test-Driven Development (TDD): write failing tests marked with `pytest.mark.xfail` before implementation, then remove the marker once the implementation is complete
Applied to files:
autogpt_platform/backend/backend/copilot/sdk/file_tools_test.py
📚 Learning: 2026-04-08T17:28:23.422Z
Learnt from: CR
Repo: Significant-Gravitas/AutoGPT PR: 0
File: autogpt_platform/backend/AGENTS.md:0-0
Timestamp: 2026-04-08T17:28:23.422Z
Learning: Applies to autogpt_platform/backend/**/*_test.py : Mock at boundaries — mock where the symbol is **used**, not where it's **defined**; after refactoring, update mock targets to match new module paths
Applied to files:
autogpt_platform/backend/backend/copilot/sdk/file_tools_test.py
🔇 Additional comments (5)
autogpt_platform/backend/backend/copilot/prompting.py (1)
78-83: Guidance update is aligned with the new truncation failure mode.This wording now matches the runtime behavior introduced in the unified MCP Write flow and gives actionable prevention guidance.
autogpt_platform/backend/backend/copilot/sdk/tool_adapter.py (2)
680-691: Blocking SDK built-inWriteis the right move here.This cleanly forces all write calls through the unified MCP
Writepath with truncation-aware handling.
628-642: No action needed — Write and Read tools are not duplicated.E2B_FILE_TOOLS contains lowercase tool names (
read_file,write_file,edit_file,glob,grep), while the standalone tools added at lines 628–642 and 644–651 are registered with capitalized names (Write,Read). These are distinct tool names and cannot conflict. Similarly, the tool name lists at lines 746–749 enumerate both sets correctly without duplication.autogpt_platform/backend/backend/copilot/sdk/file_tools_test.py (1)
22-196: Test coverage for the new Write behavior is solid.The suite exercises schema order/required fields, truncation modes, path safety, and large-content warning behavior in a focused way.
autogpt_platform/backend/backend/copilot/sdk/file_tools.py (1)
62-68: Truncation guard helper is clean and correctly scoped.The partial vs complete truncation split is straightforward and matches the intended behavior.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## dev #12750 +/- ##
==========================================
+ Coverage 63.14% 63.30% +0.16%
==========================================
Files 1811 1811
Lines 130463 131099 +636
Branches 14260 14302 +42
==========================================
+ Hits 82376 82993 +617
- Misses 45495 45497 +2
- Partials 2592 2609 +17
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
…xisting lint - Update test_write_within_workspace_allowed -> test_write_builtin_blocked to reflect that SDK built-in Write is now intentionally blocked - Fix pre-existing black formatting in platform_cost_test.py
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
…data loss Add read_file and Edit MCP tools following the same pattern as the existing unified Write tool. Both route to the E2B sandbox when active and fall back to the SDK working directory in non-E2B mode. Read tool (read_file): - Reads files with cat -n formatted line numbers - Supports offset/limit for large files - Binary file detection by extension - Path validation via is_allowed_local_path() - CLI built-in Read is NOT disabled (used internally for oversized tool results) Edit tool: - Targeted find-and-replace with old_string/new_string - replace_all flag for multi-occurrence replacements - Uniqueness check when replace_all=false - Partial truncation detection with actionable guidance - CLI built-in Edit IS disabled in SDK_DISALLOWED_TOOLS Both tools are registered in create_copilot_mcp_server() and included in get_copilot_tool_names() for both E2B and non-E2B modes.
majdyz
changed the title
The Edit tool is now blocked in SDK_DISALLOWED_TOOLS (same as Write), so the security hooks test must assert denial instead of allowing it.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
autogpt_platform/backend/backend/copilot/sdk/file_tools.py (2)
113-114:⚠️ Potential issue | 🟡 MinorSanitize write failure path before returning error text.
Line [114] returns
resolved(absolute host path), which can leak internal directory structure.As per coding guidelines, "Sanitize error paths by using `os.path.basename()` in error messages to avoid leaking directory structure".🧹 Proposed fix
- except Exception as exc: - return _mcp(f"Failed to write {resolved}: {exc}", error=True) + except Exception as exc: + safe_name = os.path.basename(file_path) or "target file" + return _mcp(f"Failed to write {safe_name}: {exc}", error=True)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@autogpt_platform/backend/backend/copilot/sdk/file_tools.py` around lines 113 - 114, The error return currently exposes the absolute path via the variable `resolved`; update the write-failure path to sanitize the path by using `os.path.basename(resolved)` (or similar) in the message passed to `_mcp` so only the filename is logged, and ensure `import os` is present in the module; change the `except Exception exc` handler that returns `_mcp(f"Failed to write {resolved}: {exc}", error=True)` to use the basename instead.
102-112:⚠️ Potential issue | 🔴 CriticalClose TOCTOU gap between validation and file write in non-E2B handlers.
Line [102] and Line [350] validate first, but Line [111] and Line [382] open/write later. A symlink swap between those points can bypass the original boundary check.
As per coding guidelines, "Be aware of TOCTOU (Time-Of-Check-Time-Of-Use) issues — avoid check-then-act patterns for file access and credit charging".🔒 Suggested hardening pattern
+def _secure_open_for_write(path: str, sdk_cwd: str) -> tuple[int | None, dict[str, Any] | None]: + flags = os.O_WRONLY | os.O_CREAT | os.O_TRUNC + if hasattr(os, "O_NOFOLLOW"): + flags |= os.O_NOFOLLOW + fd = os.open(path, flags, 0o644) + try: + actual = os.path.realpath(f"/proc/self/fd/{fd}") + if not is_allowed_local_path(actual, sdk_cwd): + os.close(fd) + return None, _mcp("Path must be within the working directory", error=True) + return fd, None + except Exception: + os.close(fd) + raiseApply this helper where files are written (Write/Edit), instead of plain
open(...).Also applies to: 350-353, 381-383
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@autogpt_platform/backend/backend/copilot/sdk/file_tools.py` around lines 102 - 112, The write path currently validates the resolved path with _resolve_and_validate then later opens the file, leaving a TOCTOU window; fix by writing via an atomic temp-then-rename pattern: after calling _resolve_and_validate (in the same function that currently calls open(resolved, "w")), create a secure temporary file in the same parent directory (use tempfile.NamedTemporaryFile or mkstemp with dir=parent, delete=False), write the content to that temp file, fsync the file, close it, then atomically replace the target with os.replace(temp_path, resolved); ensure parent dir exists beforehand and remove the temp file on errors. Apply the same change to the other write/edit sites referenced (the blocks around the later open calls noted in the comment).
🧹 Nitpick comments (2)
autogpt_platform/backend/backend/copilot/sdk/file_tools.py (2)
325-388: Refactor_handle_edit_non_e2binto smaller helpers.This function now mixes truncation checks, path validation, read logic, replacement policy, and write logic in one block; splitting improves testability and keeps control flow clearer.
As per coding guidelines, "Keep functions under ~40 lines; extract named helpers when a function grows longer".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@autogpt_platform/backend/backend/copilot/sdk/file_tools.py` around lines 325 - 388, The _handle_edit_non_e2b function is too long and mixes concerns (truncation checks, path resolution, file read/write, replacement logic); extract focused helper functions to simplify and improve testability: create helpers like _validate_edit_args(args) to handle empty file_path/old_string/truncation responses, _read_sdk_file(resolved) to encapsulate the try/except read and return content or an _mcp error, _apply_replacement(content, old_string, new_string, replace_all) to perform occurrence counting and replacement policy (return error messages via _mcp when no match or multiple matches without replace_all), and _write_sdk_file(resolved, updated) to encapsulate write errors; keep _handle_edit_non_e2b to orchestrate get_sdk_cwd(), call _resolve_and_validate(file_path, sdk_cwd), then call these helpers in sequence and return their _mcp results so behavior (including uses of _resolve_and_validate, get_sdk_cwd, and _mcp) is unchanged.
1-452: Consider splittingfile_tools.pyby tool responsibility.The module currently bundles Write/Read/Edit logic, schemas, and helper utilities in one large file. Splitting by responsibility would reduce cognitive load and make ownership boundaries clearer.
As per coding guidelines, "Keep files under ~300 lines; if a file grows beyond this, split by responsibility (extract helpers, models, or a sub-module into a new file)".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@autogpt_platform/backend/backend/copilot/sdk/file_tools.py` around lines 1 - 452, The file is too large and should be split by responsibility: extract Write, Read and Edit logic plus schemas into separate modules and factor shared utilities into a helpers module. Move the write-specific symbols (_handle_write_non_e2b, _handle_write_e2b, get_write_tool_handler, WRITE_TOOL_NAME, WRITE_TOOL_DESCRIPTION, WRITE_TOOL_SCHEMA) into a write module; move read-specific symbols (_handle_read_non_e2b, _handle_read_e2b, get_read_tool_handler, READ_TOOL_NAME, READ_TOOL_DESCRIPTION, READ_TOOL_SCHEMA, _READ_BINARY_EXTENSIONS, _is_likely_binary) into a read module; move edit-specific symbols (_handle_edit_non_e2b, _handle_edit_e2b, get_edit_tool_handler, EDIT_TOOL_NAME, EDIT_TOOL_DESCRIPTION, EDIT_TOOL_SCHEMA, _EDIT_PARTIAL_TRUNCATION_MSG) into an edit module; and place shared helpers and constants (_mcp, _check_truncation, _resolve_and_validate, get_sdk_cwd/is_allowed_local_path imports, _LARGE_CONTENT_WARN_CHARS, logger) into a common helpers module that each tool module imports. Ensure public get_*_tool_handler functions keep the same signatures so callers need minimal changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@autogpt_platform/backend/backend/copilot/sdk/file_tools.py`:
- Around line 231-232: The code directly casts args.get("offset") and
args.get("limit") to int which will raise ValueError for non-integer inputs;
wrap the parsing of offset and limit in a safe conversion in the same scope
where offset and limit are set (referencing the offset and limit variables and
args.get), e.g., try/except around int(...) to fall back to the defaults (0 for
offset, 2000 for limit) and enforce the existing clamps (offset >= 0 and limit
>= 1); ensure any parsing error is handled locally so the broader file-read
error handling still runs.
---
Duplicate comments:
In `@autogpt_platform/backend/backend/copilot/sdk/file_tools.py`:
- Around line 113-114: The error return currently exposes the absolute path via
the variable `resolved`; update the write-failure path to sanitize the path by
using `os.path.basename(resolved)` (or similar) in the message passed to `_mcp`
so only the filename is logged, and ensure `import os` is present in the module;
change the `except Exception exc` handler that returns `_mcp(f"Failed to write
{resolved}: {exc}", error=True)` to use the basename instead.
- Around line 102-112: The write path currently validates the resolved path with
_resolve_and_validate then later opens the file, leaving a TOCTOU window; fix by
writing via an atomic temp-then-rename pattern: after calling
_resolve_and_validate (in the same function that currently calls open(resolved,
"w")), create a secure temporary file in the same parent directory (use
tempfile.NamedTemporaryFile or mkstemp with dir=parent, delete=False), write the
content to that temp file, fsync the file, close it, then atomically replace the
target with os.replace(temp_path, resolved); ensure parent dir exists beforehand
and remove the temp file on errors. Apply the same change to the other
write/edit sites referenced (the blocks around the later open calls noted in the
comment).
---
Nitpick comments:
In `@autogpt_platform/backend/backend/copilot/sdk/file_tools.py`:
- Around line 325-388: The _handle_edit_non_e2b function is too long and mixes
concerns (truncation checks, path resolution, file read/write, replacement
logic); extract focused helper functions to simplify and improve testability:
create helpers like _validate_edit_args(args) to handle empty
file_path/old_string/truncation responses, _read_sdk_file(resolved) to
encapsulate the try/except read and return content or an _mcp error,
_apply_replacement(content, old_string, new_string, replace_all) to perform
occurrence counting and replacement policy (return error messages via _mcp when
no match or multiple matches without replace_all), and _write_sdk_file(resolved,
updated) to encapsulate write errors; keep _handle_edit_non_e2b to orchestrate
get_sdk_cwd(), call _resolve_and_validate(file_path, sdk_cwd), then call these
helpers in sequence and return their _mcp results so behavior (including uses of
_resolve_and_validate, get_sdk_cwd, and _mcp) is unchanged.
- Around line 1-452: The file is too large and should be split by
responsibility: extract Write, Read and Edit logic plus schemas into separate
modules and factor shared utilities into a helpers module. Move the
write-specific symbols (_handle_write_non_e2b, _handle_write_e2b,
get_write_tool_handler, WRITE_TOOL_NAME, WRITE_TOOL_DESCRIPTION,
WRITE_TOOL_SCHEMA) into a write module; move read-specific symbols
(_handle_read_non_e2b, _handle_read_e2b, get_read_tool_handler, READ_TOOL_NAME,
READ_TOOL_DESCRIPTION, READ_TOOL_SCHEMA, _READ_BINARY_EXTENSIONS,
_is_likely_binary) into a read module; move edit-specific symbols
(_handle_edit_non_e2b, _handle_edit_e2b, get_edit_tool_handler, EDIT_TOOL_NAME,
EDIT_TOOL_DESCRIPTION, EDIT_TOOL_SCHEMA, _EDIT_PARTIAL_TRUNCATION_MSG) into an
edit module; and place shared helpers and constants (_mcp, _check_truncation,
_resolve_and_validate, get_sdk_cwd/is_allowed_local_path imports,
_LARGE_CONTENT_WARN_CHARS, logger) into a common helpers module that each tool
module imports. Ensure public get_*_tool_handler functions keep the same
signatures so callers need minimal changes.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 84cdf98c-6422-42af-8d80-831f983b2a01
📒 Files selected for processing (4)
autogpt_platform/backend/backend/copilot/sdk/file_tools.pyautogpt_platform/backend/backend/copilot/sdk/file_tools_test.pyautogpt_platform/backend/backend/copilot/sdk/security_hooks_test.pyautogpt_platform/backend/backend/copilot/sdk/tool_adapter.py
✅ Files skipped from review due to trivial changes (1)
- autogpt_platform/backend/backend/copilot/sdk/file_tools_test.py
🚧 Files skipped from review as they are similar to previous changes (2)
- autogpt_platform/backend/backend/copilot/sdk/security_hooks_test.py
- autogpt_platform/backend/backend/copilot/sdk/tool_adapter.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: end-to-end tests
- GitHub Check: test (3.11)
- GitHub Check: test (3.13)
- GitHub Check: test (3.12)
- GitHub Check: Seer Code Review
- GitHub Check: Check PR Status
🧰 Additional context used
📓 Path-based instructions (2)
autogpt_platform/backend/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
autogpt_platform/backend/**/*.py: Use Python 3.11 (required; managed by Poetry via pyproject.toml) for backend development
Always run 'poetry run format' (Black + isort) before linting in backend development
Always run 'poetry run lint' (ruff) after formatting in backend development
autogpt_platform/backend/**/*.py: Usepoetry run ...command for executing Python package dependencies
Use top-level imports only — avoid local/inner imports except for lazy imports of heavy optional dependencies likeopenpyxl
Use absolute imports withfrom backend.module import ...for cross-package imports; single-dot relative imports are acceptable for sibling modules within the same package; avoid double-dot relative imports
Do not use duck typing — avoidhasattr/getattr/isinstancefor type dispatch; use typed interfaces/unions/protocols instead
Use Pydantic models over dataclass/namedtuple/dict for structured data
Do not use linter suppressors — no# type: ignore,# noqa,# pyright: ignore; fix the type/code instead
Prefer list comprehensions over manual loop-and-append patterns
Use early return with guard clauses first to avoid deep nesting
Use%sfor deferred interpolation indebuglog statements for efficiency; use f-strings elsewhere for readability (e.g.,logger.debug("Processing %s items", count)vslogger.info(f"Processing {count} items"))
Sanitize error paths by usingos.path.basename()in error messages to avoid leaking directory structure
Be aware of TOCTOU (Time-Of-Check-Time-Of-Use) issues — avoid check-then-act patterns for file access and credit charging
Usetransaction=Truefor Redis pipelines to ensure atomicity on multi-step operations
Usemax(0, value)guards for computed values that should never be negative
Keep files under ~300 lines; if a file grows beyond this, split by responsibility (extract helpers, models, or a sub-module into a new file)
Keep functions under ~40 lines; extract named helpers when a function grows longer
...
Files:
autogpt_platform/backend/backend/copilot/sdk/file_tools.py
autogpt_platform/{backend,autogpt_libs}/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Format Python code with
poetry run format
Files:
autogpt_platform/backend/backend/copilot/sdk/file_tools.py
🧠 Learnings (17)
📓 Common learnings
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12632
File: autogpt_platform/backend/backend/copilot/tools/workspace_files.py:0-0
Timestamp: 2026-04-01T04:17:43.495Z
Learning: In autogpt_platform/backend/backend/copilot/sdk/tool_adapter.py (PR `#12632`, commit 12ae03c), the per-tool `BaseTool.read_only` property approach was removed. Instead, `readOnlyHint=True` (via `ToolAnnotations`) is applied unconditionally to ALL tools — including side-effect tools like `bash_exec` and `write_workspace_file` — to enable fully parallel dispatch by the Anthropic SDK/CLI. Do not flag tools with mutating operations (e.g. save_to_path, write operations) for having `readOnlyHint=True`; this is intentional and E2E validated (3x bash_exec(sleep 3) completed in 3.3s vs 9s sequential).
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12213
File: autogpt_platform/frontend/src/app/api/openapi.json:9983-9995
Timestamp: 2026-02-27T15:59:00.370Z
Learning: Repo: Significant-Gravitas/AutoGPT PR: 12213 — Backend/frontend OpenAPI codegen
Learning: For MCP schema models, required OpenAPI fields must have no defaults in Pydantic. Specifically, MCPToolInfo.input_schema must be required (no Field(default_factory=dict)) so openapi.json emits it in "required", ensuring generated TS types treat input_schema as non-optional.
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12213
File: autogpt_platform/frontend/src/app/api/openapi.json:9983-9995
Timestamp: 2026-02-27T15:59:00.370Z
Learning: Repo: Significant-Gravitas/AutoGPT PR: 12213 — OpenAPI/codegen
Learning: Ensuring a field is required in generated TS types needs two sides: (1) no default value on the Pydantic field, and (2) the OpenAPI model's "required" array must list it. For MCPToolInfo, making input_schema required in OpenAPI and removing Field(default_factory=dict) in the backend prevents optional typing drift.
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12213
File: autogpt_platform/frontend/src/app/api/openapi.json:10030-10037
Timestamp: 2026-03-01T07:59:02.311Z
Learning: Repo: Significant-Gravitas/AutoGPT PR: 12213 — For MCP manual token storage, backend model autogpt_platform/backend/backend/api/features/mcp/routes.py defines MCPStoreTokenRequest.token as Pydantic SecretStr with a min length constraint, which generates OpenAPI schema metadata (format: "password", writeOnly: true, minLength: 1) in autogpt_platform/frontend/src/app/api/openapi.json. Prefer SecretStr (with length constraints) for sensitive request fields so generated TS clients and docs treat them as secrets.
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12213
File: autogpt_platform/frontend/src/app/(platform)/copilot/tools/RunMCPTool/helpers.tsx:23-24
Timestamp: 2026-02-27T10:45:55.700Z
Learning: As of PR `#12213`, MCP tool response types (MCPToolsDiscoveredResponse, MCPToolOutputResponse) are defined in openapi.json and frontend code in autogpt_platform/frontend/src/app/(platform)/copilot/tools/RunMCPTool/helpers.tsx uses the generated types from `@/app/api/__generated__/`. Other tools like RunBlock still use inline TypeScript interfaces (e.g., BlockDetailsResponse) for SSE stream payloads that are not included in openapi.json schemas. The pattern is tool-specific: use generated types when available in openapi.json, use inline types only when the payload schema is truly SSE-stream-only and not exposed via OpenAPI.
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12632
File: autogpt_platform/backend/backend/copilot/sdk/tool_adapter.py:530-535
Timestamp: 2026-04-01T04:17:38.279Z
Learning: In `autogpt_platform/backend/backend/copilot/sdk/tool_adapter.py`, the `ToolAnnotations(readOnlyHint=True)` annotation (stored as `_PARALLEL_ANNOTATION`) is intentionally applied to ALL registered MCP tools — including E2B write/edit tools (e.g., `write_file`, `edit_file`). This is a parallel-dispatch hint to the Claude Agent SDK CLI, not a semantic read-only contract. The `_READ_ONLY_E2B_TOOLS` set was dead code and was removed in commit `12ae03c`; the constant was renamed from `_READONLY_ANNOTATION` to `_PARALLEL_ANNOTATION` in commit `c88ca88` to avoid confusion. Do not flag this as a correctness issue.
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12440
File: autogpt_platform/backend/backend/copilot/workflow_import/converter.py:0-0
Timestamp: 2026-03-17T10:57:12.953Z
Learning: In Significant-Gravitas/AutoGPT PR `#12440`, `autogpt_platform/backend/backend/copilot/workflow_import/converter.py` was fully rewritten (commit 732960e2d) to no longer make direct LLM/OpenAI API calls. The converter now builds a structured text prompt for AutoPilot/CoPilot instead. There is no `response.choices` access or any direct LLM client usage in this file. Do not flag `response.choices` access or LLM client initialization patterns as issues in this file.
Learnt from: Pwuts
Repo: Significant-Gravitas/AutoGPT PR: 12284
File: autogpt_platform/frontend/src/app/api/openapi.json:11897-11900
Timestamp: 2026-03-04T23:58:18.476Z
Learning: Repo: Significant-Gravitas/AutoGPT — PR `#12284`
Backend/frontend OpenAPI codegen convention: In backend/api/features/store/model.py, the StoreSubmission and StoreSubmissionAdminView models define submitted_at: datetime | None, changes_summary: str | None, and instructions: str | None with no default. This is intentional to produce “required but nullable” fields in OpenAPI (properties appear in required[] and use anyOf [type, null]). This matches Prisma’s submittedAt DateTime? and changesSummary String?. Do not flag this as a required/nullable mismatch.
📚 Learning: 2026-04-01T04:17:41.600Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12632
File: autogpt_platform/backend/backend/copilot/tools/workspace_files.py:0-0
Timestamp: 2026-04-01T04:17:41.600Z
Learning: When reviewing AutoGPT Copilot tool implementations, accept that `readOnlyHint=True` (provided via `ToolAnnotations`) may be applied unconditionally to *all* tools—even tools that have side effects (e.g., `bash_exec`, `write_workspace_file`, or other write/save operations). Do **not** flag these tools for having `readOnlyHint=True`; this is intentional to enable fully-parallel dispatch by the Anthropic SDK/CLI and has been E2E validated. Only flag `readOnlyHint` issues if they conflict with the established `ToolAnnotations` behavior (e.g., missing/incorrect propagation relative to the intended annotation mechanism).
Applied to files:
autogpt_platform/backend/backend/copilot/sdk/file_tools.py
📚 Learning: 2026-04-01T04:17:38.279Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12632
File: autogpt_platform/backend/backend/copilot/sdk/tool_adapter.py:530-535
Timestamp: 2026-04-01T04:17:38.279Z
Learning: In `autogpt_platform/backend/backend/copilot/sdk/tool_adapter.py`, the `ToolAnnotations(readOnlyHint=True)` annotation (stored as `_PARALLEL_ANNOTATION`) is intentionally applied to ALL registered MCP tools — including E2B write/edit tools (e.g., `write_file`, `edit_file`). This is a parallel-dispatch hint to the Claude Agent SDK CLI, not a semantic read-only contract. The `_READ_ONLY_E2B_TOOLS` set was dead code and was removed in commit `12ae03c`; the constant was renamed from `_READONLY_ANNOTATION` to `_PARALLEL_ANNOTATION` in commit `c88ca88` to avoid confusion. Do not flag this as a correctness issue.
Applied to files:
autogpt_platform/backend/backend/copilot/sdk/file_tools.py
📚 Learning: 2026-02-27T10:45:55.700Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12213
File: autogpt_platform/frontend/src/app/(platform)/copilot/tools/RunMCPTool/helpers.tsx:23-24
Timestamp: 2026-02-27T10:45:55.700Z
Learning: As of PR `#12213`, MCP tool response types (MCPToolsDiscoveredResponse, MCPToolOutputResponse) are defined in openapi.json and frontend code in autogpt_platform/frontend/src/app/(platform)/copilot/tools/RunMCPTool/helpers.tsx uses the generated types from `@/app/api/__generated__/`. Other tools like RunBlock still use inline TypeScript interfaces (e.g., BlockDetailsResponse) for SSE stream payloads that are not included in openapi.json schemas. The pattern is tool-specific: use generated types when available in openapi.json, use inline types only when the payload schema is truly SSE-stream-only and not exposed via OpenAPI.
Applied to files:
autogpt_platform/backend/backend/copilot/sdk/file_tools.py
📚 Learning: 2026-03-17T10:57:12.953Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12440
File: autogpt_platform/backend/backend/copilot/workflow_import/converter.py:0-0
Timestamp: 2026-03-17T10:57:12.953Z
Learning: In Significant-Gravitas/AutoGPT PR `#12440`, `autogpt_platform/backend/backend/copilot/workflow_import/converter.py` was fully rewritten (commit 732960e2d) to no longer make direct LLM/OpenAI API calls. The converter now builds a structured text prompt for AutoPilot/CoPilot instead. There is no `response.choices` access or any direct LLM client usage in this file. Do not flag `response.choices` access or LLM client initialization patterns as issues in this file.
Applied to files:
autogpt_platform/backend/backend/copilot/sdk/file_tools.py
📚 Learning: 2026-02-27T15:59:00.370Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12213
File: autogpt_platform/frontend/src/app/api/openapi.json:9983-9995
Timestamp: 2026-02-27T15:59:00.370Z
Learning: Repo: Significant-Gravitas/AutoGPT PR: 12213 — Backend/frontend OpenAPI codegen
Learning: For MCP schema models, required OpenAPI fields must have no defaults in Pydantic. Specifically, MCPToolInfo.input_schema must be required (no Field(default_factory=dict)) so openapi.json emits it in "required", ensuring generated TS types treat input_schema as non-optional.
Applied to files:
autogpt_platform/backend/backend/copilot/sdk/file_tools.py
📚 Learning: 2026-02-27T15:59:00.370Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12213
File: autogpt_platform/frontend/src/app/api/openapi.json:9983-9995
Timestamp: 2026-02-27T15:59:00.370Z
Learning: Repo: Significant-Gravitas/AutoGPT PR: 12213 — OpenAPI/codegen
Learning: Ensuring a field is required in generated TS types needs two sides: (1) no default value on the Pydantic field, and (2) the OpenAPI model's "required" array must list it. For MCPToolInfo, making input_schema required in OpenAPI and removing Field(default_factory=dict) in the backend prevents optional typing drift.
Applied to files:
autogpt_platform/backend/backend/copilot/sdk/file_tools.py
📚 Learning: 2026-04-08T17:28:23.422Z
Learnt from: CR
Repo: Significant-Gravitas/AutoGPT PR: 0
File: autogpt_platform/backend/AGENTS.md:0-0
Timestamp: 2026-04-08T17:28:23.422Z
Learning: Applies to autogpt_platform/backend/**/*.py : Be aware of TOCTOU (Time-Of-Check-Time-Of-Use) issues — avoid check-then-act patterns for file access and credit charging
Applied to files:
autogpt_platform/backend/backend/copilot/sdk/file_tools.py
📚 Learning: 2026-04-08T17:27:45.725Z
Learnt from: CR
Repo: Significant-Gravitas/AutoGPT PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-08T17:27:45.725Z
Learning: Applies to autogpt_platform/**/data/**/*.py : For changes touching `data/*.py`, validate user ID checks or explain why not needed
Applied to files:
autogpt_platform/backend/backend/copilot/sdk/file_tools.py
📚 Learning: 2026-04-08T17:28:23.422Z
Learnt from: CR
Repo: Significant-Gravitas/AutoGPT PR: 0
File: autogpt_platform/backend/AGENTS.md:0-0
Timestamp: 2026-04-08T17:28:23.422Z
Learning: Applies to autogpt_platform/backend/**/*.py : Sanitize error paths by using `os.path.basename()` in error messages to avoid leaking directory structure
Applied to files:
autogpt_platform/backend/backend/copilot/sdk/file_tools.py
📚 Learning: 2026-04-03T13:53:33.653Z
Learnt from: Pwuts
Repo: Significant-Gravitas/AutoGPT PR: 12206
File: autogpt_platform/backend/snapshots/v2_unhandled_exception_500:1-5
Timestamp: 2026-04-03T13:53:33.653Z
Learning: In Significant-Gravitas/AutoGPT (autogpt_platform), the catch-all `Exception` handler in `autogpt_platform/backend/backend/api/utils/exceptions.py` (`_handle_error()`) intentionally surfaces `str(exc)` as the `detail` field in HTTP 500 responses for non-Prisma errors. This is by design: errors are logged server-side, and the detail helps API consumers report issues. Only `PrismaError` responses are sanitized (see commit ce6910b4a). Do not flag `str(exc)` in the generic 500 handler as an information disclosure issue; the snapshot `autogpt_platform/backend/snapshots/v2_unhandled_exception_500` ("connection refused") correctly reflects this behavior.
Applied to files:
autogpt_platform/backend/backend/copilot/sdk/file_tools.py
📚 Learning: 2026-03-16T16:30:30.764Z
Learnt from: Abhi1992002
Repo: Significant-Gravitas/AutoGPT PR: 12417
File: autogpt_platform/backend/backend/blocks/agent_mail/pods.py:62-74
Timestamp: 2026-03-16T16:30:30.764Z
Learning: In autogpt_platform/backend/backend/blocks/**/*.py, explicit try/except in the `run()` method is NOT required for standard error handling. The block framework's `_execute()` method in `_base.py` catches unhandled exceptions and re-raises them as `BlockExecutionError` or `BlockUnknownError`. Additionally, when a block yields `("error", message)`, `_execute()` immediately raises `BlockExecutionError` — so the `error` output port never propagates downstream. Explicit try/except is only needed when partial output must be controlled (e.g., attachment blocks that must skip yielding `content_base64` on failure).
Applied to files:
autogpt_platform/backend/backend/copilot/sdk/file_tools.py
📚 Learning: 2026-02-26T17:02:22.448Z
Learnt from: Pwuts
Repo: Significant-Gravitas/AutoGPT PR: 12211
File: .pre-commit-config.yaml:160-179
Timestamp: 2026-02-26T17:02:22.448Z
Learning: Keep the pre-commit hook pattern broad for autogpt_platform/backend to ensure OpenAPI schema changes are captured. Do not narrow to backend/api/ alone, since the generated schema depends on Pydantic models across multiple directories (backend/data/, backend/blocks/, backend/copilot/, backend/integrations/, backend/util/). Narrowing could miss schema changes and cause frontend type desynchronization.
Applied to files:
autogpt_platform/backend/backend/copilot/sdk/file_tools.py
📚 Learning: 2026-03-04T08:04:35.881Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12273
File: autogpt_platform/backend/backend/copilot/tools/workspace_files.py:216-220
Timestamp: 2026-03-04T08:04:35.881Z
Learning: In the AutoGPT Copilot backend, ensure that SVG images are not treated as vision image types by excluding 'image/svg+xml' from INLINEABLE_MIME_TYPES and MULTIMODAL_TYPES in tool_adapter.py; the Claude API supports PNG, JPEG, GIF, and WebP for vision. SVGs (XML text) should be handled via the text path instead, not the vision path.
Applied to files:
autogpt_platform/backend/backend/copilot/sdk/file_tools.py
📚 Learning: 2026-03-05T15:42:08.207Z
Learnt from: ntindle
Repo: Significant-Gravitas/AutoGPT PR: 12297
File: .claude/skills/backend-check/SKILL.md:14-16
Timestamp: 2026-03-05T15:42:08.207Z
Learning: In Python files under autogpt_platform/backend (recursively), rely on poetry run format to perform formatting (Black + isort) and linting (ruff). Do not run poetry run lint as a separate step after poetry run format, since format already includes linting checks.
Applied to files:
autogpt_platform/backend/backend/copilot/sdk/file_tools.py
📚 Learning: 2026-03-16T16:35:40.236Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12440
File: autogpt_platform/backend/backend/api/features/workflow_import.py:54-63
Timestamp: 2026-03-16T16:35:40.236Z
Learning: Avoid using the word 'competitor' in public-facing identifiers and text. Use neutral naming for API paths, model names, function names, and UI text. Examples: rename 'CompetitorFormat' to 'SourcePlatform', 'convert_competitor_workflow' to 'convert_workflow', '/competitor-workflow' to '/workflow'. Apply this guideline to files under autogpt_platform/backend and autogpt_platform/frontend.
Applied to files:
autogpt_platform/backend/backend/copilot/sdk/file_tools.py
📚 Learning: 2026-03-31T15:37:38.626Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12623
File: autogpt_platform/backend/backend/copilot/tools/agent_generator/fixer.py:37-47
Timestamp: 2026-03-31T15:37:38.626Z
Learning: When validating/constructing Anthropic API model IDs in Significant-Gravitas/AutoGPT, allow the hyphen-separated Claude Opus 4.6 model ID `claude-opus-4-6` (it corresponds to `LlmModel.CLAUDE_4_6_OPUS` in `autogpt_platform/backend/backend/blocks/llm.py`). Do NOT require the dot-separated form in Anthropic contexts. Only OpenRouter routing variants should use the dot separator (e.g., `anthropic/claude-opus-4.6`); `claude-opus-4-6` should be treated as correct when passed to Anthropic, and flagged only if it’s used in the OpenRouter path where the dot form is expected.
Applied to files:
autogpt_platform/backend/backend/copilot/sdk/file_tools.py
🔇 Additional comments (1)
autogpt_platform/backend/backend/copilot/sdk/file_tools.py (1)
59-65: Solid truncation detection and actionable UX messaging.The partial/complete truncation checks are clear and guide recovery paths well.
Also applies to: 317-341, 398-407
- Sanitize error-path output in write failures: use os.path.basename() instead of exposing full resolved path in error messages - Validate offset/limit parsing in read_file: wrap int() calls in try/except to return clean error on non-integer input - Fix duplicate read_file registration in E2B mode: skip unified read_file when use_e2b=True since E2B_FILE_TOOLS already registers it - Add tests for invalid offset/limit input
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
autogpt_platform/backend/backend/copilot/sdk/file_tools.py (2)
262-267:⚠️ Potential issue | 🟡 MinorSanitize read/edit error paths too.
These branches still echo
file_pathdirectly, so reusing an absolute path from a prior success response will leak the full workspace path in the next error. Useos.path.basename(...)consistently in user-facing read/edit failures, like the Write handler already does. As per coding guidelines, "Sanitize error paths by usingos.path.basename()in error messages to avoid leaking directory structure".Also applies to: 365-369, 390-391
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@autogpt_platform/backend/backend/copilot/sdk/file_tools.py` around lines 262 - 267, The exception handlers in the read/edit flows (e.g., in functions read_file and edit_file) currently include the raw file_path in user-facing error messages and must be sanitized; update the FileNotFoundError, PermissionError, and generic Exception branches to use os.path.basename(file_path) instead of file_path when calling _mcp so directory structure isn't leaked, and ensure import os is present at the top of the module; apply the same change to the other similar handlers referenced (the edit handler and the related read/write exception blocks).
68-86:⚠️ Potential issue | 🟠 MajorMake validation and file open a single step.
is_allowed_local_path()resolves symlinks during validation, but these handlers later open the returned path string separately. A symlink swap between those steps can still let Read/Write/Edit escapesdk_cwd; use an open-then-verify flow instead of returning a pre-validated path. As per coding guidelines, "Be aware of TOCTOU (Time-Of-Check-Time-Of-Use) issues — avoid check-then-act patterns for file access and credit charging".Also applies to: 107-112, 259-260, 361-363, 387-389
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@autogpt_platform/backend/backend/copilot/sdk/file_tools.py` around lines 68 - 86, The current _resolve_and_validate + is_allowed_local_path flow is a TOCTOU risk because callers open the returned path after validation; change it to open-then-verify and return an opened handle (or an fd) instead of a raw path: resolve sdk_cwd to its realpath, open the target with os.open using appropriate flags (O_RDONLY/O_RDWR) and then verify the opened fd's realpath (via os.readlink/f'/proc/self/fd/{fd}' or os.fstat + os.path.realpath on that link) is still within the sdk_cwd realpath using os.path.commonpath; if the check fails, close the fd and return the error via _mcp; update callers that expect a path to accept the file descriptor or file object (or add a small wrapper that returns a safe tempfile-like object) and apply the same open-then-verify fix to the other sites that perform check-then-open (the other validate+open call paths referenced alongside _resolve_and_validate and is_allowed_local_path).
🧹 Nitpick comments (1)
autogpt_platform/backend/backend/copilot/sdk/file_tools.py (1)
1-458: Split this module by tool before it grows further.At ~460 lines, this already exceeds the backend size guideline and packs three handlers, three schemas, and shared helpers into one file. Extracting per-tool modules plus a small shared path/truncation helper would make future changes much easier to reason about. As per coding guidelines, "Keep files under ~300 lines; if a file grows beyond this, split by responsibility (extract helpers, models, or a sub-module into a new file)".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@autogpt_platform/backend/backend/copilot/sdk/file_tools.py` around lines 1 - 458, Split this large module into per-tool modules and a small shared helper module: move Write-related symbols (get_write_tool_handler, _handle_write_non_e2b, _handle_write_e2b, WRITE_TOOL_NAME, WRITE_TOOL_DESCRIPTION, WRITE_TOOL_SCHEMA, _LARGE_CONTENT_WARN_CHARS) into a write_tool module; move Read-related symbols (get_read_tool_handler, _handle_read_non_e2b, _handle_read_e2b, READ_TOOL_NAME, READ_TOOL_DESCRIPTION, READ_TOOL_SCHEMA, _READ_BINARY_EXTENSIONS, _is_likely_binary) into a read_tool module; move Edit-related symbols (get_edit_tool_handler, _handle_edit_non_e2b, _handle_edit_e2b, EDIT_TOOL_NAME, EDIT_TOOL_DESCRIPTION, EDIT_TOOL_SCHEMA, _EDIT_PARTIAL_TRUNCATION_MSG) into an edit_tool module; extract shared helpers (_mcp, _check_truncation, _resolve_and_validate and any constants they use) into a small file_helpers (or utils) module that imports get_sdk_cwd and is_allowed_local_path where needed; update imports in the new modules to reference the helpers and the e2b delegate imports (_handle_write_file/_handle_read_file/_handle_edit_file) and ensure unit tests / exports import the new entry-point functions (get_*_tool_handler) instead of the original combined module.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@autogpt_platform/backend/backend/copilot/sdk/file_tools.py`:
- Around line 228-241: In _handle_read_non_e2b add the same truncation-recovery
guard used for Write/Edit: detect when file_path is falsy but args contains
offset or limit (or both) and return a specific _mcp truncation error advising
the caller/model that the request was likely truncated and to resend the full
file_path (instead of the generic "file_path is required" message); update the
branch in the _handle_read_non_e2b function to check for presence of "offset" or
"limit" before returning the _mcp("file_path is required", ...) so the model
gets an actionable recovery path.
- Around line 361-389: The Edit routine performs an uncoordinated
read-modify-write (see variables resolved, content, updated and the write block)
and can lose parallel updates; protect it by adding same-path coordination:
create a per-path lock map (keyed by resolved) and acquire the lock around the
read-modify-write, or implement optimistic compare-before-write by re-reading
the file just before writing and aborting/returning an error via _mcp if the
on-disk content differs from the earlier read; when writing use an atomic
replace (temp file + os.replace) to avoid partial writes and ensure _mcp returns
a clear conflict message if the compare fails.
---
Duplicate comments:
In `@autogpt_platform/backend/backend/copilot/sdk/file_tools.py`:
- Around line 262-267: The exception handlers in the read/edit flows (e.g., in
functions read_file and edit_file) currently include the raw file_path in
user-facing error messages and must be sanitized; update the FileNotFoundError,
PermissionError, and generic Exception branches to use
os.path.basename(file_path) instead of file_path when calling _mcp so directory
structure isn't leaked, and ensure import os is present at the top of the
module; apply the same change to the other similar handlers referenced (the edit
handler and the related read/write exception blocks).
- Around line 68-86: The current _resolve_and_validate + is_allowed_local_path
flow is a TOCTOU risk because callers open the returned path after validation;
change it to open-then-verify and return an opened handle (or an fd) instead of
a raw path: resolve sdk_cwd to its realpath, open the target with os.open using
appropriate flags (O_RDONLY/O_RDWR) and then verify the opened fd's realpath
(via os.readlink/f'/proc/self/fd/{fd}' or os.fstat + os.path.realpath on that
link) is still within the sdk_cwd realpath using os.path.commonpath; if the
check fails, close the fd and return the error via _mcp; update callers that
expect a path to accept the file descriptor or file object (or add a small
wrapper that returns a safe tempfile-like object) and apply the same
open-then-verify fix to the other sites that perform check-then-open (the other
validate+open call paths referenced alongside _resolve_and_validate and
is_allowed_local_path).
---
Nitpick comments:
In `@autogpt_platform/backend/backend/copilot/sdk/file_tools.py`:
- Around line 1-458: Split this large module into per-tool modules and a small
shared helper module: move Write-related symbols (get_write_tool_handler,
_handle_write_non_e2b, _handle_write_e2b, WRITE_TOOL_NAME,
WRITE_TOOL_DESCRIPTION, WRITE_TOOL_SCHEMA, _LARGE_CONTENT_WARN_CHARS) into a
write_tool module; move Read-related symbols (get_read_tool_handler,
_handle_read_non_e2b, _handle_read_e2b, READ_TOOL_NAME, READ_TOOL_DESCRIPTION,
READ_TOOL_SCHEMA, _READ_BINARY_EXTENSIONS, _is_likely_binary) into a read_tool
module; move Edit-related symbols (get_edit_tool_handler, _handle_edit_non_e2b,
_handle_edit_e2b, EDIT_TOOL_NAME, EDIT_TOOL_DESCRIPTION, EDIT_TOOL_SCHEMA,
_EDIT_PARTIAL_TRUNCATION_MSG) into an edit_tool module; extract shared helpers
(_mcp, _check_truncation, _resolve_and_validate and any constants they use) into
a small file_helpers (or utils) module that imports get_sdk_cwd and
is_allowed_local_path where needed; update imports in the new modules to
reference the helpers and the e2b delegate imports
(_handle_write_file/_handle_read_file/_handle_edit_file) and ensure unit tests /
exports import the new entry-point functions (get_*_tool_handler) instead of the
original combined module.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 21f92ef8-2eb7-4a14-bfce-f9bacade01ce
📒 Files selected for processing (3)
autogpt_platform/backend/backend/copilot/sdk/file_tools.pyautogpt_platform/backend/backend/copilot/sdk/file_tools_test.pyautogpt_platform/backend/backend/copilot/sdk/tool_adapter.py
✅ Files skipped from review due to trivial changes (1)
- autogpt_platform/backend/backend/copilot/sdk/file_tools_test.py
🚧 Files skipped from review as they are similar to previous changes (1)
- autogpt_platform/backend/backend/copilot/sdk/tool_adapter.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: check API types
- GitHub Check: Seer Code Review
- GitHub Check: test (3.13)
- GitHub Check: test (3.11)
- GitHub Check: test (3.12)
- GitHub Check: end-to-end tests
- GitHub Check: Check PR Status
- GitHub Check: Analyze (python)
- GitHub Check: Analyze (typescript)
🧰 Additional context used
📓 Path-based instructions (2)
autogpt_platform/backend/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
autogpt_platform/backend/**/*.py: Use Python 3.11 (required; managed by Poetry via pyproject.toml) for backend development
Always run 'poetry run format' (Black + isort) before linting in backend development
Always run 'poetry run lint' (ruff) after formatting in backend development
autogpt_platform/backend/**/*.py: Usepoetry run ...command for executing Python package dependencies
Use top-level imports only — avoid local/inner imports except for lazy imports of heavy optional dependencies likeopenpyxl
Use absolute imports withfrom backend.module import ...for cross-package imports; single-dot relative imports are acceptable for sibling modules within the same package; avoid double-dot relative imports
Do not use duck typing — avoidhasattr/getattr/isinstancefor type dispatch; use typed interfaces/unions/protocols instead
Use Pydantic models over dataclass/namedtuple/dict for structured data
Do not use linter suppressors — no# type: ignore,# noqa,# pyright: ignore; fix the type/code instead
Prefer list comprehensions over manual loop-and-append patterns
Use early return with guard clauses first to avoid deep nesting
Use%sfor deferred interpolation indebuglog statements for efficiency; use f-strings elsewhere for readability (e.g.,logger.debug("Processing %s items", count)vslogger.info(f"Processing {count} items"))
Sanitize error paths by usingos.path.basename()in error messages to avoid leaking directory structure
Be aware of TOCTOU (Time-Of-Check-Time-Of-Use) issues — avoid check-then-act patterns for file access and credit charging
Usetransaction=Truefor Redis pipelines to ensure atomicity on multi-step operations
Usemax(0, value)guards for computed values that should never be negative
Keep files under ~300 lines; if a file grows beyond this, split by responsibility (extract helpers, models, or a sub-module into a new file)
Keep functions under ~40 lines; extract named helpers when a function grows longer
...
Files:
autogpt_platform/backend/backend/copilot/sdk/file_tools.py
autogpt_platform/{backend,autogpt_libs}/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Format Python code with
poetry run format
Files:
autogpt_platform/backend/backend/copilot/sdk/file_tools.py
🧠 Learnings (18)
📓 Common learnings
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12632
File: autogpt_platform/backend/backend/copilot/tools/workspace_files.py:0-0
Timestamp: 2026-04-01T04:17:43.495Z
Learning: In autogpt_platform/backend/backend/copilot/sdk/tool_adapter.py (PR `#12632`, commit 12ae03c), the per-tool `BaseTool.read_only` property approach was removed. Instead, `readOnlyHint=True` (via `ToolAnnotations`) is applied unconditionally to ALL tools — including side-effect tools like `bash_exec` and `write_workspace_file` — to enable fully parallel dispatch by the Anthropic SDK/CLI. Do not flag tools with mutating operations (e.g. save_to_path, write operations) for having `readOnlyHint=True`; this is intentional and E2E validated (3x bash_exec(sleep 3) completed in 3.3s vs 9s sequential).
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12213
File: autogpt_platform/frontend/src/app/api/openapi.json:9983-9995
Timestamp: 2026-02-27T15:59:00.370Z
Learning: Repo: Significant-Gravitas/AutoGPT PR: 12213 — Backend/frontend OpenAPI codegen
Learning: For MCP schema models, required OpenAPI fields must have no defaults in Pydantic. Specifically, MCPToolInfo.input_schema must be required (no Field(default_factory=dict)) so openapi.json emits it in "required", ensuring generated TS types treat input_schema as non-optional.
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12213
File: autogpt_platform/frontend/src/app/api/openapi.json:9983-9995
Timestamp: 2026-02-27T15:59:00.370Z
Learning: Repo: Significant-Gravitas/AutoGPT PR: 12213 — OpenAPI/codegen
Learning: Ensuring a field is required in generated TS types needs two sides: (1) no default value on the Pydantic field, and (2) the OpenAPI model's "required" array must list it. For MCPToolInfo, making input_schema required in OpenAPI and removing Field(default_factory=dict) in the backend prevents optional typing drift.
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12632
File: autogpt_platform/backend/backend/copilot/sdk/tool_adapter.py:530-535
Timestamp: 2026-04-01T04:17:38.279Z
Learning: In `autogpt_platform/backend/backend/copilot/sdk/tool_adapter.py`, the `ToolAnnotations(readOnlyHint=True)` annotation (stored as `_PARALLEL_ANNOTATION`) is intentionally applied to ALL registered MCP tools — including E2B write/edit tools (e.g., `write_file`, `edit_file`). This is a parallel-dispatch hint to the Claude Agent SDK CLI, not a semantic read-only contract. The `_READ_ONLY_E2B_TOOLS` set was dead code and was removed in commit `12ae03c`; the constant was renamed from `_READONLY_ANNOTATION` to `_PARALLEL_ANNOTATION` in commit `c88ca88` to avoid confusion. Do not flag this as a correctness issue.
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12440
File: autogpt_platform/backend/backend/copilot/workflow_import/converter.py:0-0
Timestamp: 2026-03-17T10:57:12.953Z
Learning: In Significant-Gravitas/AutoGPT PR `#12440`, `autogpt_platform/backend/backend/copilot/workflow_import/converter.py` was fully rewritten (commit 732960e2d) to no longer make direct LLM/OpenAI API calls. The converter now builds a structured text prompt for AutoPilot/CoPilot instead. There is no `response.choices` access or any direct LLM client usage in this file. Do not flag `response.choices` access or LLM client initialization patterns as issues in this file.
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12213
File: autogpt_platform/frontend/src/app/(platform)/copilot/tools/RunMCPTool/helpers.tsx:23-24
Timestamp: 2026-02-27T10:45:55.700Z
Learning: As of PR `#12213`, MCP tool response types (MCPToolsDiscoveredResponse, MCPToolOutputResponse) are defined in openapi.json and frontend code in autogpt_platform/frontend/src/app/(platform)/copilot/tools/RunMCPTool/helpers.tsx uses the generated types from `@/app/api/__generated__/`. Other tools like RunBlock still use inline TypeScript interfaces (e.g., BlockDetailsResponse) for SSE stream payloads that are not included in openapi.json schemas. The pattern is tool-specific: use generated types when available in openapi.json, use inline types only when the payload schema is truly SSE-stream-only and not exposed via OpenAPI.
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12213
File: autogpt_platform/frontend/src/app/api/openapi.json:10030-10037
Timestamp: 2026-03-01T07:59:02.311Z
Learning: Repo: Significant-Gravitas/AutoGPT PR: 12213 — For MCP manual token storage, backend model autogpt_platform/backend/backend/api/features/mcp/routes.py defines MCPStoreTokenRequest.token as Pydantic SecretStr with a min length constraint, which generates OpenAPI schema metadata (format: "password", writeOnly: true, minLength: 1) in autogpt_platform/frontend/src/app/api/openapi.json. Prefer SecretStr (with length constraints) for sensitive request fields so generated TS clients and docs treat them as secrets.
Learnt from: ntindle
Repo: Significant-Gravitas/AutoGPT PR: 12536
File: autogpt_platform/frontend/src/app/api/openapi.json:5770-5790
Timestamp: 2026-03-24T21:25:15.983Z
Learning: Repo: Significant-Gravitas/AutoGPT — PR `#12536`
File: autogpt_platform/frontend/src/app/api/openapi.json
Learning: The OpenAPI spec file is auto-generated; per established convention, endpoints generally declare only 200/201, 401, and 422 responses. Do not suggest adding explicit 403/404 response entries for single operations unless planning a repo-wide spec update. Prefer clarifying such behaviors in endpoint descriptions/docstrings instead of altering response maps.
📚 Learning: 2026-04-01T04:17:41.600Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12632
File: autogpt_platform/backend/backend/copilot/tools/workspace_files.py:0-0
Timestamp: 2026-04-01T04:17:41.600Z
Learning: When reviewing AutoGPT Copilot tool implementations, accept that `readOnlyHint=True` (provided via `ToolAnnotations`) may be applied unconditionally to *all* tools—even tools that have side effects (e.g., `bash_exec`, `write_workspace_file`, or other write/save operations). Do **not** flag these tools for having `readOnlyHint=True`; this is intentional to enable fully-parallel dispatch by the Anthropic SDK/CLI and has been E2E validated. Only flag `readOnlyHint` issues if they conflict with the established `ToolAnnotations` behavior (e.g., missing/incorrect propagation relative to the intended annotation mechanism).
Applied to files:
autogpt_platform/backend/backend/copilot/sdk/file_tools.py
📚 Learning: 2026-04-01T04:17:38.279Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12632
File: autogpt_platform/backend/backend/copilot/sdk/tool_adapter.py:530-535
Timestamp: 2026-04-01T04:17:38.279Z
Learning: In `autogpt_platform/backend/backend/copilot/sdk/tool_adapter.py`, the `ToolAnnotations(readOnlyHint=True)` annotation (stored as `_PARALLEL_ANNOTATION`) is intentionally applied to ALL registered MCP tools — including E2B write/edit tools (e.g., `write_file`, `edit_file`). This is a parallel-dispatch hint to the Claude Agent SDK CLI, not a semantic read-only contract. The `_READ_ONLY_E2B_TOOLS` set was dead code and was removed in commit `12ae03c`; the constant was renamed from `_READONLY_ANNOTATION` to `_PARALLEL_ANNOTATION` in commit `c88ca88` to avoid confusion. Do not flag this as a correctness issue.
Applied to files:
autogpt_platform/backend/backend/copilot/sdk/file_tools.py
📚 Learning: 2026-03-17T10:57:12.953Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12440
File: autogpt_platform/backend/backend/copilot/workflow_import/converter.py:0-0
Timestamp: 2026-03-17T10:57:12.953Z
Learning: In Significant-Gravitas/AutoGPT PR `#12440`, `autogpt_platform/backend/backend/copilot/workflow_import/converter.py` was fully rewritten (commit 732960e2d) to no longer make direct LLM/OpenAI API calls. The converter now builds a structured text prompt for AutoPilot/CoPilot instead. There is no `response.choices` access or any direct LLM client usage in this file. Do not flag `response.choices` access or LLM client initialization patterns as issues in this file.
Applied to files:
autogpt_platform/backend/backend/copilot/sdk/file_tools.py
📚 Learning: 2026-02-27T10:45:55.700Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12213
File: autogpt_platform/frontend/src/app/(platform)/copilot/tools/RunMCPTool/helpers.tsx:23-24
Timestamp: 2026-02-27T10:45:55.700Z
Learning: As of PR `#12213`, MCP tool response types (MCPToolsDiscoveredResponse, MCPToolOutputResponse) are defined in openapi.json and frontend code in autogpt_platform/frontend/src/app/(platform)/copilot/tools/RunMCPTool/helpers.tsx uses the generated types from `@/app/api/__generated__/`. Other tools like RunBlock still use inline TypeScript interfaces (e.g., BlockDetailsResponse) for SSE stream payloads that are not included in openapi.json schemas. The pattern is tool-specific: use generated types when available in openapi.json, use inline types only when the payload schema is truly SSE-stream-only and not exposed via OpenAPI.
Applied to files:
autogpt_platform/backend/backend/copilot/sdk/file_tools.py
📚 Learning: 2026-02-27T15:59:00.370Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12213
File: autogpt_platform/frontend/src/app/api/openapi.json:9983-9995
Timestamp: 2026-02-27T15:59:00.370Z
Learning: Repo: Significant-Gravitas/AutoGPT PR: 12213 — Backend/frontend OpenAPI codegen
Learning: For MCP schema models, required OpenAPI fields must have no defaults in Pydantic. Specifically, MCPToolInfo.input_schema must be required (no Field(default_factory=dict)) so openapi.json emits it in "required", ensuring generated TS types treat input_schema as non-optional.
Applied to files:
autogpt_platform/backend/backend/copilot/sdk/file_tools.py
📚 Learning: 2026-02-27T15:59:00.370Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12213
File: autogpt_platform/frontend/src/app/api/openapi.json:9983-9995
Timestamp: 2026-02-27T15:59:00.370Z
Learning: Repo: Significant-Gravitas/AutoGPT PR: 12213 — OpenAPI/codegen
Learning: Ensuring a field is required in generated TS types needs two sides: (1) no default value on the Pydantic field, and (2) the OpenAPI model's "required" array must list it. For MCPToolInfo, making input_schema required in OpenAPI and removing Field(default_factory=dict) in the backend prevents optional typing drift.
Applied to files:
autogpt_platform/backend/backend/copilot/sdk/file_tools.py
📚 Learning: 2026-04-08T17:28:23.422Z
Learnt from: CR
Repo: Significant-Gravitas/AutoGPT PR: 0
File: autogpt_platform/backend/AGENTS.md:0-0
Timestamp: 2026-04-08T17:28:23.422Z
Learning: Applies to autogpt_platform/backend/**/*.py : Be aware of TOCTOU (Time-Of-Check-Time-Of-Use) issues — avoid check-then-act patterns for file access and credit charging
Applied to files:
autogpt_platform/backend/backend/copilot/sdk/file_tools.py
📚 Learning: 2026-04-08T17:27:45.725Z
Learnt from: CR
Repo: Significant-Gravitas/AutoGPT PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-08T17:27:45.725Z
Learning: Applies to autogpt_platform/**/data/**/*.py : For changes touching `data/*.py`, validate user ID checks or explain why not needed
Applied to files:
autogpt_platform/backend/backend/copilot/sdk/file_tools.py
📚 Learning: 2026-04-08T17:28:23.422Z
Learnt from: CR
Repo: Significant-Gravitas/AutoGPT PR: 0
File: autogpt_platform/backend/AGENTS.md:0-0
Timestamp: 2026-04-08T17:28:23.422Z
Learning: Applies to autogpt_platform/backend/**/*.py : Sanitize error paths by using `os.path.basename()` in error messages to avoid leaking directory structure
Applied to files:
autogpt_platform/backend/backend/copilot/sdk/file_tools.py
📚 Learning: 2026-04-03T13:53:33.653Z
Learnt from: Pwuts
Repo: Significant-Gravitas/AutoGPT PR: 12206
File: autogpt_platform/backend/snapshots/v2_unhandled_exception_500:1-5
Timestamp: 2026-04-03T13:53:33.653Z
Learning: In Significant-Gravitas/AutoGPT (autogpt_platform), the catch-all `Exception` handler in `autogpt_platform/backend/backend/api/utils/exceptions.py` (`_handle_error()`) intentionally surfaces `str(exc)` as the `detail` field in HTTP 500 responses for non-Prisma errors. This is by design: errors are logged server-side, and the detail helps API consumers report issues. Only `PrismaError` responses are sanitized (see commit ce6910b4a). Do not flag `str(exc)` in the generic 500 handler as an information disclosure issue; the snapshot `autogpt_platform/backend/snapshots/v2_unhandled_exception_500` ("connection refused") correctly reflects this behavior.
Applied to files:
autogpt_platform/backend/backend/copilot/sdk/file_tools.py
📚 Learning: 2026-04-08T17:28:23.422Z
Learnt from: CR
Repo: Significant-Gravitas/AutoGPT PR: 0
File: autogpt_platform/backend/AGENTS.md:0-0
Timestamp: 2026-04-08T17:28:23.422Z
Learning: Applies to autogpt_platform/backend/**/*.py : Use `max(0, value)` guards for computed values that should never be negative
Applied to files:
autogpt_platform/backend/backend/copilot/sdk/file_tools.py
📚 Learning: 2026-03-16T17:00:02.827Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12439
File: autogpt_platform/backend/backend/blocks/autogpt_copilot.py:0-0
Timestamp: 2026-03-16T17:00:02.827Z
Learning: In autogpt_platform/backend/backend/blocks/autogpt_copilot.py, the recursion guard uses two module-level ContextVars: `_copilot_recursion_depth` (tracks current nesting depth) and `_copilot_recursion_limit` (stores the chain-wide ceiling). On the first invocation, `_copilot_recursion_limit` is set to `max_recursion_depth`; nested calls use `min(inherited_limit, max_recursion_depth)`, so they can only lower the cap, never raise it. The entry/exit logic is extracted into module-level helper functions. This is the approved pattern for preventing runaway sub-agent recursion in AutogptCopilotBlock (PR `#12439`, commits 348e9f8e2 and 3b70f61b1).
Applied to files:
autogpt_platform/backend/backend/copilot/sdk/file_tools.py
📚 Learning: 2026-02-26T17:02:22.448Z
Learnt from: Pwuts
Repo: Significant-Gravitas/AutoGPT PR: 12211
File: .pre-commit-config.yaml:160-179
Timestamp: 2026-02-26T17:02:22.448Z
Learning: Keep the pre-commit hook pattern broad for autogpt_platform/backend to ensure OpenAPI schema changes are captured. Do not narrow to backend/api/ alone, since the generated schema depends on Pydantic models across multiple directories (backend/data/, backend/blocks/, backend/copilot/, backend/integrations/, backend/util/). Narrowing could miss schema changes and cause frontend type desynchronization.
Applied to files:
autogpt_platform/backend/backend/copilot/sdk/file_tools.py
📚 Learning: 2026-03-04T08:04:35.881Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12273
File: autogpt_platform/backend/backend/copilot/tools/workspace_files.py:216-220
Timestamp: 2026-03-04T08:04:35.881Z
Learning: In the AutoGPT Copilot backend, ensure that SVG images are not treated as vision image types by excluding 'image/svg+xml' from INLINEABLE_MIME_TYPES and MULTIMODAL_TYPES in tool_adapter.py; the Claude API supports PNG, JPEG, GIF, and WebP for vision. SVGs (XML text) should be handled via the text path instead, not the vision path.
Applied to files:
autogpt_platform/backend/backend/copilot/sdk/file_tools.py
📚 Learning: 2026-03-05T15:42:08.207Z
Learnt from: ntindle
Repo: Significant-Gravitas/AutoGPT PR: 12297
File: .claude/skills/backend-check/SKILL.md:14-16
Timestamp: 2026-03-05T15:42:08.207Z
Learning: In Python files under autogpt_platform/backend (recursively), rely on poetry run format to perform formatting (Black + isort) and linting (ruff). Do not run poetry run lint as a separate step after poetry run format, since format already includes linting checks.
Applied to files:
autogpt_platform/backend/backend/copilot/sdk/file_tools.py
📚 Learning: 2026-03-16T16:35:40.236Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12440
File: autogpt_platform/backend/backend/api/features/workflow_import.py:54-63
Timestamp: 2026-03-16T16:35:40.236Z
Learning: Avoid using the word 'competitor' in public-facing identifiers and text. Use neutral naming for API paths, model names, function names, and UI text. Examples: rename 'CompetitorFormat' to 'SourcePlatform', 'convert_competitor_workflow' to 'convert_workflow', '/competitor-workflow' to '/workflow'. Apply this guideline to files under autogpt_platform/backend and autogpt_platform/frontend.
Applied to files:
autogpt_platform/backend/backend/copilot/sdk/file_tools.py
📚 Learning: 2026-03-31T15:37:38.626Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12623
File: autogpt_platform/backend/backend/copilot/tools/agent_generator/fixer.py:37-47
Timestamp: 2026-03-31T15:37:38.626Z
Learning: When validating/constructing Anthropic API model IDs in Significant-Gravitas/AutoGPT, allow the hyphen-separated Claude Opus 4.6 model ID `claude-opus-4-6` (it corresponds to `LlmModel.CLAUDE_4_6_OPUS` in `autogpt_platform/backend/backend/blocks/llm.py`). Do NOT require the dot-separated form in Anthropic contexts. Only OpenRouter routing variants should use the dot separator (e.g., `anthropic/claude-opus-4.6`); `claude-opus-4-6` should be treated as correct when passed to Anthropic, and flagged only if it’s used in the OpenRouter path where the dot form is expected.
Applied to files:
autogpt_platform/backend/backend/copilot/sdk/file_tools.py
- Add truncation recovery to read_file: when offset/limit are present but file_path is missing, return actionable truncation error instead of generic "file_path is required" - Add per-path asyncio lock around Edit's read-modify-write cycle to prevent parallel edits on the same file from silently dropping changes - Add tests for read truncation detection
Clean up _edit_locks entries after the edit completes when no other coroutine is waiting on the same path. Prevents unbounded growth of the lock dictionary in long-running server deployments.
4 tasks
majdyz
commented
Apr 13, 2026
majdyz
commented
Apr 13, 2026
…r edit lock - Replace os.path.normpath with os.path.realpath in _resolve_and_validate so symlinks within sdk_cwd pointing outside the allowed directory are caught at path resolution time (normpath does not follow symlinks) - Add asyncio.sleep(0) between read and write phases of _handle_edit_file so the event loop can schedule other coroutines, making the per-path lock a genuine regression guard against concurrent edit races - Expand test docstring to document the sleep(0) yield mechanism
…prevent memory leak
- Sanitize raw absolute paths in e2b_file_tools.py error messages with os.path.basename() - Narrow security_hooks.py Read carve-out to only SDK artifact paths (tool-results/, tool-outputs/) - Fix dead code in tool_adapter.py _make_truncating_wrapper: detect empty args via schema properties instead of required (which _build_input_schema intentionally omits)
7 tasks
… tool restriction Read tool was restricted to SDK artifact paths (tool-results/ tool-outputs/) only; workspace reads must use the read_file MCP tool. Update test to assert denial for workspace file access, consistent with the security hook behavior.
Contributor
Author
|
🤖 Fixed CI test failure in 318f7b8: |
Contributor
Author
🧪 E2E Test Result — preview/all-active-prsResult: PASS
Screenshots
|
…id leaking sandbox paths
23 tasks
…ost sdk_cwd
In E2B mode, relative file paths (e.g. "output.txt") were passing the
_is_allowed_local() check (resolving against sdk_cwd on the host) and
being read from the host filesystem instead of the sandbox. Files written
by the agent's Write/Edit tools in E2B mode go to the sandbox, so a
subsequent read_file("output.txt") would fail with "File not found".
Fix: add is_sdk_tool_path() to context.py that only matches SDK-internal
tool-results/tool-outputs paths. When E2B is active, use is_sdk_tool_path
instead of _is_allowed_local so that only genuine SDK-internal paths stay
on the host; all other paths are routed to the sandbox.
Add TestReadFileE2BRouting tests to verify the routing is correct.
…ion only - Replace `_is_sdk_artifact_path()` in security_hooks.py with shared `is_sdk_tool_path()` from context.py, which validates that the path's encoded-cwd segment matches the current session's _current_project_dir, preventing cross-session tool-results reads. - Use `is_sdk_tool_path()` in `_read_file_handler` (read_tool_result MCP tool) instead of the broader `is_allowed_local_path()`, restricting the MCP tool to tool-results/tool-outputs paths only. - Add cross-session test to security_hooks_test.py verifying that session A cannot read session B's tool-results directory. - Update tool_adapter_test.py patches to use new is_sdk_tool_path.
…tricted read_tool_result read_tool_result now only accepts SDK tool-results/tool-outputs paths (not sdk_cwd paths), so test_read_file_handler_local_file is updated to verify that sdk_cwd files are rejected with "path not allowed" error.
This was referenced Apr 14, 2026
github-project-automation
Bot
moved this from 🆕 Needs initial review
to ✅ Done
in AutoGPT development kanban
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why / What / How
Why: The Claude Agent SDK's built-in Write and Edit tools have no defence against output-token truncation. When the LLM generates a large
contentornew_stringargument, the API truncates the response mid-JSON, causing Ajv to reject it with the opaque"'file_path' is a required property"error. The user's work is silently lost, and retrying with the same approach loops infinitely.What: Replaces the SDK's built-in Write and Edit tools with unified MCP equivalents that detect truncation and return actionable recovery guidance. Adds a new
read_fileMCP tool with offset/limit pagination. Consolidates all file-tool handlers into a single module (e2b_file_tools.py) covering both E2B (sandbox) and non-E2B (local SDK working directory) modes.How:
file_pathis placed first in every JSON schema so truncation is more likely to preserve the path"required"is intentionally omitted from all MCP schemas so the MCP SDK delivers empty/truncated args to the handler instead of rejecting them with an opaque error{}) and partial (other fields present butfile_pathmissing), returning actionable error messages in both casesasyncio.Lock(keyed by resolved absolute path) to prevent parallel read-modify-write races when MCP tools are dispatched concurrentlyis_allowed_local_path()/is_within_allowed_dirs()to block directory traversalSDK_DISALLOWED_TOOLS; the SDK built-in Read remains allowed only for workspace-scoped paths (tool-results/tool-outputs) viaWORKSPACE_SCOPED_TOOLSreadOnlyHint=False(_MUTATING_ANNOTATION) to prevent parallel dispatchbridge_to_sandboxcopies host-side tool-result files into the E2B sandbox on read sobash_execcan process themChanges 🏗️
e2b_file_tools.py— unified file-tool handlers for Write, Read (read_file), Edit, Glob, Grep covering both E2B and non-E2B modes; per-path edit locking; truncation detection; sandbox symlink-escape check;bridge_to_sandboxfor SDK→E2B file bridgingtool_adapter.py— registers unified Write/Edit/read_file MCP tools (non-E2B only); addsReadtool for workspace-scoped SDK-internal reads (both modes); E2B tools use_MUTATING_ANNOTATION;get_copilot_tool_names/get_sdk_disallowed_toolsupdated for both modessecurity_hooks.py—WORKSPACE_SCOPED_TOOLSchecked beforeBLOCKED_TOOLSso SDK internal Read is allowed on tool-results paths; Write/Edit removed from workspace scopeprompting.py— improved wording for large-file truncation warninge2b_file_tools_test.py— comprehensive tests for non-E2B Write/Read/Edit (path validation, truncation detection, offset/limit, binary rejection, schema validation); E2B sandbox symlink-escape,bridge_to_sandbox, and_sandbox_writetestssecurity_hooks_test.py— updated tests for revised tool-blocking and workspace-scoped Read behaviourChecklist 📋
For code changes:
SDK_DISALLOWED_TOOLS; Read is workspace-scoped only_MUTATING_ANNOTATION(not parallel)black,ruff,pyrightpass on all modified files