Skip to content

fix(backend/copilot): fix tool output file reading between E2B and host - #12646

Merged
majdyz merged 17 commits into
devfrom
fix/copilot-tool-output-e2b-bridging
Apr 2, 2026
Merged

fix(backend/copilot): fix tool output file reading between E2B and host#12646
majdyz merged 17 commits into
devfrom
fix/copilot-tool-output-e2b-bridging

Conversation

@majdyz

@majdyz majdyz commented Apr 2, 2026

Copy link
Copy Markdown
Contributor

Why / What / How

Why: When copilot tools return large outputs (e.g. 3MB+ base64 images from API calls), the agent cannot process them in the E2B sandbox. Three compounding issues prevent seamless file access:

  1. The <tool-output-truncated path="..."> tag uses a bare path= attribute that the model confuses with a local filesystem path (it's actually a workspace path)
  2. is_allowed_local_path rejects tool-outputs/ directories (only tool-results/ was allowed)
  3. SDK-internal files read via the Read tool are not available in the E2B sandbox for bash_exec processing

What: Fixes all three issues so that large tool outputs can be seamlessly read and processed in both host and E2B contexts.

How:

  • Changed path=workspace_path= in the truncation tag to disambiguate workspace vs filesystem paths
  • Added save_to_path guidance in the retrieval instructions for E2B users
  • Extended is_allowed_local_path to accept both tool-results and tool-outputs directories
  • Added automatic bridging: when E2B is active and Read accesses an SDK-internal file, the file is automatically copied to /tmp/<filename> in the sandbox
  • Updated system prompting to explain both SDK tool-result bridging and workspace <tool-output-truncated> handling

Changes 🏗️

  • tools/base.py: _persist_and_summarize now uses workspace_path= attribute and includes save_to_path example for E2B processing
  • context.py: is_allowed_local_path accepts both tool-results and tool-outputs directory names
  • sdk/e2b_file_tools.py: _handle_read_file bridges SDK-internal files to /tmp/ in E2B sandbox; new _bridge_to_sandbox helper
  • prompting.py: Updated "SDK tool-result files" section and added "Large tool outputs saved to workspace" section
  • Tests: Added tool-outputs path validation tests in context_test.py and e2b_file_tools_test.py; updated base_test.py assertion for workspace_path

Checklist 📋

For code changes:

  • I have clearly listed my changes in the PR description
  • I have made a test plan
  • I have tested my changes according to the test plan:
    • poetry run pytest backend/copilot/tools/base_test.py — all 9 tests pass (persistence, truncation, binary fields)
    • poetry run format and poetry run lint pass clean
    • All pre-commit hooks pass
    • context_test.py, e2b_file_tools_test.py, security_hooks_test.py — blocked by pre-existing DB migration issue on worktree (missing User.subscriptionTier column); CI will validate these

Three issues prevented the copilot agent from processing large tool
outputs (e.g. base64 images) in the E2B sandbox:

1. _persist_and_summarize used path= attribute in the truncation tag,
   which the model confused with a local filesystem path. Changed to
   workspace_path= and added save_to_path guidance for E2B processing.

2. is_allowed_local_path only accepted "tool-results" directory but the
   SDK may also use "tool-outputs". Now accepts both.

3. When E2B is active and the Read tool accesses an SDK-internal file,
   the content was returned to the conversation but not available in
   the sandbox for bash processing. Added automatic bridging that copies
   the file into /tmp/<filename> in the sandbox.
@majdyz
majdyz requested a review from a team as a code owner April 2, 2026 05:48
@github-project-automation github-project-automation Bot moved this to 🆕 Needs initial review in AutoGPT development kanban Apr 2, 2026
@majdyz
majdyz requested review from 0ubbe and Swiftyos and removed request for a team April 2, 2026 05:48
@majdyz

majdyz commented Apr 2, 2026

Copy link
Copy Markdown
Contributor Author

/review

@github-actions github-actions Bot added the platform/backend AutoGPT Platform - Back end label Apr 2, 2026
@autogpt-pr-reviewer

Copy link
Copy Markdown

Queued a review for PR #12646 at f3dd708.

@coderabbitai

coderabbitai Bot commented Apr 2, 2026

Copy link
Copy Markdown
Contributor

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

Broadened host-path allowlist to accept tool-outputs alongside tool-results; added best-effort E2B sandbox bridging to copy eligible host files into active sandboxes for bash_exec access; updated CoPilot prompts and persistence metadata to use workspace_path; tests expanded for tool-outputs and bridging behavior.

Changes

Cohort / File(s) Summary
Path validation & tests
autogpt_platform/backend/backend/copilot/context.py, autogpt_platform/backend/backend/copilot/context_test.py, autogpt_platform/backend/backend/copilot/sdk/security_hooks_test.py
is_allowed_local_path() now accepts either tool-results or tool-outputs as the UUID subdirectory; test cases and docstrings updated to cover tool-outputs.
E2B sandbox bridging & tests
autogpt_platform/backend/backend/copilot/sdk/e2b_file_tools.py, autogpt_platform/backend/backend/copilot/sdk/e2b_file_tools_test.py, autogpt_platform/backend/backend/copilot/sdk/tool_adapter.py
Added _bridge_to_sandbox() with offset/limit gating and size thresholds; _handle_read_file / _read_file_handler invoke bridging when a sandbox exists and append a “[Sandbox copy available at …]” marker on success; tests cover gating, size-based skips, error swallowing, and sandbox write paths.
Tool persistence & tests
autogpt_platform/backend/backend/copilot/tools/base.py, autogpt_platform/backend/backend/copilot/tools/base_test.py
_persist_and_summarize() now emits workspace_path="..." and includes sandbox-processing retrieval instructions; corresponding test expectations updated.
Prompting & docs
autogpt_platform/backend/backend/copilot/prompting.py
Expanded system prompt and supplements to require saving binary/base64 outputs to workspace (write_workspace_file) and referencing with @@agptfile:; clarified truncated SDK tool-output handling and sandbox access semantics.
Test fixtures
autogpt_platform/backend/backend/copilot/sdk/conftest.py
Added async session-scoped server fixture returning None and an autouse graph_cleanup fixture to decouple SDK tests from backend dependencies.

Sequence Diagram

sequenceDiagram
    participant Client as Tool Handler
    participant Read as _handle_read_file()
    participant Validate as is_allowed_local_path()
    participant HostFS as Host Filesystem
    participant Bridge as _bridge_to_sandbox()
    participant Sandbox as E2B Sandbox
    participant BashExec as bash_exec

    Client->>Read: request read_local(`<...>/<uuid>/tool-outputs/data.json`, offset, limit)
    Read->>Validate: is_allowed_local_path(path)
    Validate->>HostFS: confirm allowed segment (tool-results/tool-outputs)
    HostFS-->>Validate: allowed
    Read->>HostFS: open & read selected bytes
    HostFS-->>Read: content
    Read->>Bridge: _bridge_to_sandbox(Sandbox, path, offset, limit)
    Bridge->>HostFS: resolve real path & stat size
    HostFS-->>Bridge: size/contents
    alt offset==0 and limit>=2000 and size within thresholds
        Bridge->>Sandbox: write file (via /tmp or /home/user)
        Sandbox-->>Bridge: ack
        Bridge-->>Read: bridged path
    else skip or error
        Bridge-->>Read: no-op (logged)
    end
    Read-->>Client: return MCP Read result (+ "[Sandbox copy available at ...]" if bridged)
    Client->>BashExec: optionally run bash_exec on bridged sandbox path
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

Suggested reviewers

  • 0ubbe
  • Swiftyos
  • Pwuts
  • ntindle

Poem

🐰 I hop from results into outputs bright,

I ferry host files into sandbox light,
workspace_path gleams where the bytes abide,
Bridged and ready for bash—come run and stride! 🎉

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: fixing tool output file reading between E2B and host environments by addressing truncation tag disambiguation, expanding allowed paths, and adding automatic sandbox bridging.
Description check ✅ Passed The description clearly explains the three underlying issues, how they are fixed, lists all affected files, and includes test status; it is directly related to the changeset.
Docstring Coverage ✅ Passed Docstring coverage is 96.43% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/copilot-tool-output-e2b-bridging

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions Bot added the size/l label Apr 2, 2026
Comment thread autogpt_platform/backend/backend/copilot/sdk/e2b_file_tools.py Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
autogpt_platform/backend/backend/copilot/tools/base.py (1)

100-102: Inline comment wording is inconsistent with actual output format.

Line 100 mentions a workspace:// prefix, but the wrapper emits workspace_path="...". Updating the comment would avoid confusion.

Suggested comment-only fix
-    # Use workspace:// prefix so the model doesn't confuse the workspace path
-    # with a local filesystem path (e.g. ~/.claude/projects/.../tool-outputs/).
+    # Use workspace_path=... so the model doesn't confuse this workspace path
+    # with a local filesystem path (e.g. ~/.claude/projects/.../tool-outputs/).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@autogpt_platform/backend/backend/copilot/tools/base.py` around lines 100 -
102, The inline comment in
autogpt_platform/backend/backend/copilot/tools/base.py incorrectly states the
wrapper uses a "workspace://" prefix while the actual wrapper emits
workspace_path="..."; update the comment above the return to describe the real
format (e.g., mention workspace_path="..." instead of workspace://) so the
wording matches the emitted output and avoids confusion when reading the code in
functions around the return.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@autogpt_platform/backend/backend/copilot/tools/base.py`:
- Around line 100-102: The inline comment in
autogpt_platform/backend/backend/copilot/tools/base.py incorrectly states the
wrapper uses a "workspace://" prefix while the actual wrapper emits
workspace_path="..."; update the comment above the return to describe the real
format (e.g., mention workspace_path="..." instead of workspace://) so the
wording matches the emitted output and avoids confusion when reading the code in
functions around the return.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 9ac38d5b-3aad-4faa-b738-6134b0b270bc

📥 Commits

Reviewing files that changed from the base of the PR and between b9e29c9 and f3dd708.

📒 Files selected for processing (8)
  • autogpt_platform/backend/backend/copilot/context.py
  • autogpt_platform/backend/backend/copilot/context_test.py
  • autogpt_platform/backend/backend/copilot/prompting.py
  • autogpt_platform/backend/backend/copilot/sdk/e2b_file_tools.py
  • autogpt_platform/backend/backend/copilot/sdk/e2b_file_tools_test.py
  • autogpt_platform/backend/backend/copilot/sdk/security_hooks_test.py
  • autogpt_platform/backend/backend/copilot/tools/base.py
  • autogpt_platform/backend/backend/copilot/tools/base_test.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: Analyze (typescript)
  • GitHub Check: Analyze (python)
  • GitHub Check: Check PR Status
🧰 Additional context used
📓 Path-based instructions (4)
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

Refer to @backend/CLAUDE.md for backend-specific commands, architecture, and development tasks

autogpt_platform/backend/**/*.py: Import only at the top level; no local/inner imports except for lazy imports of heavy optional dependencies like openpyxl
Use absolute imports with from backend.module import ... for cross-package imports; single-dot relative imports (from .sibling import ...) are acceptable for sibling modules within the same package; avoid double-dot relative imports (from ..parent import ...)
Do not use duck typing with hasattr(), getattr(), or isinstance() for type dispatch; use typed interfaces, unions, or protocols instead
Use Pydantic models for structured data instead of dataclasses, namedtuples, or dicts
Do not use linter suppressors; no # type: ignore, # noqa, or # pyright: ignore comments — fix the underlying type/code issue instead
Use list comprehensions instead of manual loop-and-append patterns
Use early return guard clauses to avoid deep nesting
Use %s for deferred interpolation in debug log statements; use f-strings for readability in other log levels (e.g., logger.debug("Processing %s items", count), logger.info(f"Processing {count} items"))
Sanitize error paths using os.path.basename() in error messages to avoid leaking directory structure
Avoid TOCTOU (time-of-check-time-of-use) patterns; do not use check-then-act patterns for file access and credit charging operations
Use Redis pipelines with transaction=True for atomicity on multi-step Redis operations
Use max(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 h...

Files:

  • autogpt_platform/backend/backend/copilot/tools/base_test.py
  • autogpt_platform/backend/backend/copilot/sdk/security_hooks_test.py
  • autogpt_platform/backend/backend/copilot/context.py
  • autogpt_platform/backend/backend/copilot/context_test.py
  • autogpt_platform/backend/backend/copilot/tools/base.py
  • autogpt_platform/backend/backend/copilot/prompting.py
  • autogpt_platform/backend/backend/copilot/sdk/e2b_file_tools.py
  • autogpt_platform/backend/backend/copilot/sdk/e2b_file_tools_test.py
autogpt_platform/**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

Format Python code with poetry run format

Files:

  • autogpt_platform/backend/backend/copilot/tools/base_test.py
  • autogpt_platform/backend/backend/copilot/sdk/security_hooks_test.py
  • autogpt_platform/backend/backend/copilot/context.py
  • autogpt_platform/backend/backend/copilot/context_test.py
  • autogpt_platform/backend/backend/copilot/tools/base.py
  • autogpt_platform/backend/backend/copilot/prompting.py
  • autogpt_platform/backend/backend/copilot/sdk/e2b_file_tools.py
  • autogpt_platform/backend/backend/copilot/sdk/e2b_file_tools_test.py
autogpt_platform/backend/**/*test*.py

📄 CodeRabbit inference engine (AGENTS.md)

Run poetry run test for backend testing (runs pytest with docker based postgres + prisma)

Files:

  • autogpt_platform/backend/backend/copilot/tools/base_test.py
  • autogpt_platform/backend/backend/copilot/sdk/security_hooks_test.py
  • autogpt_platform/backend/backend/copilot/context_test.py
  • autogpt_platform/backend/backend/copilot/sdk/e2b_file_tools_test.py
autogpt_platform/backend/**/*_test.py

📄 CodeRabbit inference engine (autogpt_platform/backend/CLAUDE.md)

autogpt_platform/backend/**/*_test.py: Use pytest with snapshot testing for API responses; test files should be colocated with source files using the *_test.py naming pattern
Mock at boundaries by mocking where the symbol is used, not where it is defined. After refactoring, update mock targets to match new module paths
Use AsyncMock from unittest.mock for mocking async functions

Files:

  • autogpt_platform/backend/backend/copilot/tools/base_test.py
  • autogpt_platform/backend/backend/copilot/sdk/security_hooks_test.py
  • autogpt_platform/backend/backend/copilot/context_test.py
  • autogpt_platform/backend/backend/copilot/sdk/e2b_file_tools_test.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:41.600Z
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: 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/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: 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.
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: 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.
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:35.195Z
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: 12356
File: autogpt_platform/backend/backend/copilot/constants.py:9-12
Timestamp: 2026-03-10T08:39:22.025Z
Learning: In Significant-Gravitas/AutoGPT PR `#12356`, the `COPILOT_SYNTHETIC_ID_PREFIX = "copilot-"` check in `create_auto_approval_record` (human_review.py) is intentional and safe. The `graph_exec_id` passed to this function comes from server-side `PendingHumanReview` DB records (not from user input); the API only accepts `node_exec_id` from users. Synthetic `copilot-*` IDs are only ever created server-side in `run_block.py`. The prefix skip avoids a DB lookup for a `AgentGraphExecution` record that legitimately does not exist for CoPilot sessions, while `user_id` scoping is enforced at the auth layer and on the resulting auto-approval record.
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12426
File: autogpt_platform/backend/backend/copilot/sdk/service.py:0-0
Timestamp: 2026-03-15T16:52:15.463Z
Learning: In Significant-Gravitas/AutoGPT (copilot backend), GitHub tokens (GH_TOKEN / GITHUB_TOKEN) for the `gh` CLI are injected lazily per-command in `autogpt_platform/backend/backend/copilot/tools/bash_exec._execute_on_e2b()` by calling `integration_creds.get_integration_env_vars(user_id)`, not on the global SDK subprocess environment in `sdk/service.py`. This scopes credentials to individual E2B sandbox command invocations and prevents token leakage into tool output streams or uploaded transcripts.
📚 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/tools/base_test.py
  • autogpt_platform/backend/backend/copilot/context_test.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/tools/base_test.py
  • autogpt_platform/backend/backend/copilot/sdk/security_hooks_test.py
  • autogpt_platform/backend/backend/copilot/context.py
  • autogpt_platform/backend/backend/copilot/context_test.py
  • autogpt_platform/backend/backend/copilot/tools/base.py
  • autogpt_platform/backend/backend/copilot/prompting.py
  • autogpt_platform/backend/backend/copilot/sdk/e2b_file_tools.py
  • autogpt_platform/backend/backend/copilot/sdk/e2b_file_tools_test.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/tools/base_test.py
  • autogpt_platform/backend/backend/copilot/sdk/security_hooks_test.py
  • autogpt_platform/backend/backend/copilot/context.py
  • autogpt_platform/backend/backend/copilot/tools/base.py
  • 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/tools/base_test.py
  • autogpt_platform/backend/backend/copilot/sdk/security_hooks_test.py
  • autogpt_platform/backend/backend/copilot/context.py
  • autogpt_platform/backend/backend/copilot/context_test.py
  • autogpt_platform/backend/backend/copilot/tools/base.py
  • autogpt_platform/backend/backend/copilot/prompting.py
  • autogpt_platform/backend/backend/copilot/sdk/e2b_file_tools.py
  • autogpt_platform/backend/backend/copilot/sdk/e2b_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/tools/base_test.py
  • autogpt_platform/backend/backend/copilot/sdk/security_hooks_test.py
  • autogpt_platform/backend/backend/copilot/context.py
  • autogpt_platform/backend/backend/copilot/context_test.py
  • autogpt_platform/backend/backend/copilot/tools/base.py
  • autogpt_platform/backend/backend/copilot/prompting.py
  • autogpt_platform/backend/backend/copilot/sdk/e2b_file_tools.py
  • autogpt_platform/backend/backend/copilot/sdk/e2b_file_tools_test.py
📚 Learning: 2026-03-04T12:19:39.243Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12279
File: autogpt_platform/backend/backend/copilot/tools/base.py:184-188
Timestamp: 2026-03-04T12:19:39.243Z
Learning: In autogpt_platform/backend/backend/copilot/tools/, ensure that anonymous users always pass user_id=None to tool execution methods. The anon_ prefix (e.g., anon_123) is used only for PostHog/analytics distinct_id and must not be used as an actual user_id. Use a simple truthiness check on user_id (e.g., if user_id: ... else: ... or a dedicated is_authenticated flag) to distinguish anonymous from authenticated users, and review all tool execution call sites within this directory to prevent accidentally forwarding an anon_ user_id to tools.

Applied to files:

  • autogpt_platform/backend/backend/copilot/tools/base_test.py
  • autogpt_platform/backend/backend/copilot/tools/base.py
📚 Learning: 2026-03-31T14:22:26.566Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12622
File: autogpt_platform/backend/backend/copilot/tools/agent_search.py:223-236
Timestamp: 2026-03-31T14:22:26.566Z
Learning: In files under autogpt_platform/backend/backend/copilot/tools/, ensure agent graph enrichment uses the typed Pydantic model `backend.data.graph.Graph` for `AgentInfo.graph` (i.e., `Graph | None`), not `dict[str, Any]`. When enriching with graph data (e.g., `_enrich_agents_with_graph`), prefer calling `graph_db().get_graph(graph_id, version=None, user_id=user_id)` directly to retrieve the typed `Graph` object rather than routing through JSON conversions like `get_agent_as_json()` / `graph_to_json()`.

Applied to files:

  • autogpt_platform/backend/backend/copilot/tools/base_test.py
  • autogpt_platform/backend/backend/copilot/tools/base.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/tools/base_test.py
  • autogpt_platform/backend/backend/copilot/sdk/security_hooks_test.py
  • autogpt_platform/backend/backend/copilot/context.py
  • autogpt_platform/backend/backend/copilot/context_test.py
  • autogpt_platform/backend/backend/copilot/tools/base.py
  • autogpt_platform/backend/backend/copilot/prompting.py
  • autogpt_platform/backend/backend/copilot/sdk/e2b_file_tools.py
  • autogpt_platform/backend/backend/copilot/sdk/e2b_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/tools/base_test.py
  • autogpt_platform/backend/backend/copilot/sdk/security_hooks_test.py
  • autogpt_platform/backend/backend/copilot/context.py
  • autogpt_platform/backend/backend/copilot/context_test.py
  • autogpt_platform/backend/backend/copilot/tools/base.py
  • autogpt_platform/backend/backend/copilot/prompting.py
  • autogpt_platform/backend/backend/copilot/sdk/e2b_file_tools.py
  • autogpt_platform/backend/backend/copilot/sdk/e2b_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/tools/base_test.py
  • autogpt_platform/backend/backend/copilot/sdk/security_hooks_test.py
  • autogpt_platform/backend/backend/copilot/context.py
  • autogpt_platform/backend/backend/copilot/context_test.py
  • autogpt_platform/backend/backend/copilot/tools/base.py
  • autogpt_platform/backend/backend/copilot/prompting.py
  • autogpt_platform/backend/backend/copilot/sdk/e2b_file_tools.py
  • autogpt_platform/backend/backend/copilot/sdk/e2b_file_tools_test.py
📚 Learning: 2026-04-01T04:17:35.195Z
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:35.195Z
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/security_hooks_test.py
  • autogpt_platform/backend/backend/copilot/prompting.py
  • autogpt_platform/backend/backend/copilot/sdk/e2b_file_tools.py
  • autogpt_platform/backend/backend/copilot/sdk/e2b_file_tools_test.py
📚 Learning: 2026-02-04T16:50:51.495Z
Learnt from: CR
Repo: Significant-Gravitas/AutoGPT PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-04T16:50:51.495Z
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/security_hooks_test.py
  • autogpt_platform/backend/backend/copilot/context.py
  • autogpt_platform/backend/backend/copilot/context_test.py
📚 Learning: 2026-03-26T07:00:03.405Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12574
File: autogpt_platform/backend/backend/copilot/sdk/transcript.py:980-990
Timestamp: 2026-03-26T07:00:03.405Z
Learning: In `autogpt_platform/backend/backend/copilot/sdk/transcript.py`, `_rechain_tail` intentionally rewrites `parentUuid` for **all** tail entries (not just the first), because a single assistant turn can span multiple consecutive JSONL entries sharing the same `message.id` (e.g., a thinking entry + a tool_use entry). Their original `parentUuid` values may reference entries that were absorbed into the compressed prefix, so sequential rechaining of the entire tail is required to maintain a valid parent→child graph. The test `test_chains_multiple_tail_entries` validates this: the second tail entry's `parentUuid` is rewritten from its original value to the uuid of the first tail entry.

Applied to files:

  • autogpt_platform/backend/backend/copilot/context_test.py
📚 Learning: 2026-03-25T06:59:27.340Z
Learnt from: CR
Repo: Significant-Gravitas/AutoGPT PR: 0
File: autogpt_platform/backend/CLAUDE.md:0-0
Timestamp: 2026-03-25T06:59:27.340Z
Learning: Applies to autogpt_platform/backend/**/*_test.py : Mock at boundaries by mocking where the symbol is used, not where it is defined. After refactoring, update mock targets to match new module paths

Applied to files:

  • autogpt_platform/backend/backend/copilot/context_test.py
📚 Learning: 2026-03-31T15:37:19.733Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12623
File: autogpt_platform/backend/backend/copilot/transcript_builder.py:30-34
Timestamp: 2026-03-31T15:37:19.733Z
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 | None` and `model_dump_json(exclude_none=True)` is used intentionally. A missing `parentUuid` in the serialized JSONL is how the CLI identifies the root entry. Do not flag `parentUuid=None` / omitted `parentUuid` as a bug — this is correct, pre-existing behavior and must not be changed to `""`.

Applied to files:

  • autogpt_platform/backend/backend/copilot/context_test.py
  • autogpt_platform/backend/backend/copilot/prompting.py
📚 Learning: 2026-03-27T09:36:59.358Z
Learnt from: ntindle
Repo: Significant-Gravitas/AutoGPT PR: 12591
File: .claude/skills/setup-repo/SKILL.md:32-33
Timestamp: 2026-03-27T09:36:59.358Z
Learning: In the Significant-Gravitas/AutoGPT repository, bash code blocks inside `.claude/skills/*/SKILL.md` files are illustrative guidance patterns for AI agents to adapt, not directly executable scripts. Code correctness standards (e.g., regex safety, error handling) for these snippets should be evaluated against their role as intent-communicating documentation rather than production code.

Applied to files:

  • autogpt_platform/backend/backend/copilot/prompting.py
📚 Learning: 2026-03-15T16:52:15.463Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12426
File: autogpt_platform/backend/backend/copilot/sdk/service.py:0-0
Timestamp: 2026-03-15T16:52:15.463Z
Learning: In Significant-Gravitas/AutoGPT (copilot backend), GitHub tokens (GH_TOKEN / GITHUB_TOKEN) for the `gh` CLI are injected lazily per-command in `autogpt_platform/backend/backend/copilot/tools/bash_exec._execute_on_e2b()` by calling `integration_creds.get_integration_env_vars(user_id)`, not on the global SDK subprocess environment in `sdk/service.py`. This scopes credentials to individual E2B sandbox command invocations and prevents token leakage into tool output streams or uploaded transcripts.

Applied to files:

  • autogpt_platform/backend/backend/copilot/prompting.py
  • autogpt_platform/backend/backend/copilot/sdk/e2b_file_tools.py
🔇 Additional comments (8)
autogpt_platform/backend/backend/copilot/tools/base_test.py (1)

70-70: Assertion update matches the new truncated-output contract.

This correctly validates the renamed metadata attribute and protects against regressions in _persist_and_summarize.

autogpt_platform/backend/backend/copilot/sdk/security_hooks_test.py (1)

140-140: Docstring clarification is accurate and helpful.

Good update to reflect the tool-results/tool-outputs scope explicitly.

autogpt_platform/backend/backend/copilot/context_test.py (1)

137-149: Coverage expansion for tool-outputs is solid.

This test/doc update nicely closes the gap after broadening is_allowed_local_path.

Also applies to: 177-177

autogpt_platform/backend/backend/copilot/context.py (1)

177-190: Allowlist broadening is implemented safely.

Keeping the UUID gate and explicit second-segment allowlist preserves the intended security boundary.

autogpt_platform/backend/backend/copilot/tools/base.py (1)

94-99: Large-output retrieval guidance is a strong improvement.

The added save_to_path flow makes downstream E2B processing much clearer.

Also applies to: 103-103

autogpt_platform/backend/backend/copilot/sdk/e2b_file_tools_test.py (1)

94-97: Great addition to validate tool-outputs host reads.

This test meaningfully improves confidence in the new allowlist behavior.

Also applies to: 130-148

autogpt_platform/backend/backend/copilot/sdk/e2b_file_tools.py (1)

136-145: Bridge integration is clean and resilient.

Good call making bridging best-effort and non-blocking so Read behavior remains stable even if copy fails.

Also applies to: 315-347

autogpt_platform/backend/backend/copilot/prompting.py (1)

214-223: Prompt guidance update is clear and actionable.

This accurately distinguishes host SDK artifacts from workspace-backed large outputs and gives concrete retrieval flows.

majdyz added 3 commits April 2, 2026 07:56
…g, tests

- Move E2B-specific bridging text from shared prompt section to E2B
  supplement's extra_notes (MAJOR 1)
- Add size cap to _bridge_to_sandbox: <=5MB uses shell base64 to /tmp,
  5-50MB uses sandbox.files.write to /home/user, >50MB skipped (MAJOR 2)
- Add 7 unit tests for _bridge_to_sandbox covering happy path, skip
  conditions, error handling, and size-based routing (MINOR 3)
- Fix inaccurate comment about tool-outputs name origin (NIT 7)
- Update is_allowed_local_path docstring to mention tool-outputs (NIT 9)
- Add prompting guidance for handling base64 images in tool outputs
  (save to workspace, show via download URL)
- Add prompting guidance for using @@agptfile: references instead of
  copy-pasting large data between tools
- Add no-op server/graph_cleanup fixtures to sdk/conftest.py so SDK
  unit tests don't require Postgres
…ng for images

- Add _bridge_to_sandbox call in _read_file_handler (tool_adapter.py)
  so the MCP Read tool (which the model actually uses) also bridges
  SDK-internal files into the E2B sandbox — not just the E2B read_file
- Move E2B-specific bridging text to _E2B_TOOL_NOTES (not shown in
  local bubblewrap mode)
- Add size-tiered bridging: shell base64 for <=5MB, files API for
  5-50MB, skip for >50MB
- Add CRITICAL prompting sections for binary/image data handling
  (use workspace, not inline) and @@agptfile references
- Add 7 unit tests for _bridge_to_sandbox
- Fix comment accuracy in context.py, update docstring
…f copy location

Address CodeRabbit review: _bridge_to_sandbox now returns the sandbox
path (or None on failure) so callers can append "[Sandbox copy available
at /tmp/file.json]" to the Read result. This gives the model explicit
feedback about where to find the file in the sandbox, instead of
silently bridging with no indication.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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/conftest.py`:
- Around line 15-21: Remove the illegal type-ignore suppressors on the fixture
functions: delete the "# type: ignore[override]" comments on the server and
graph_cleanup fixtures, give the fixtures explicit names via the
pytest_asyncio.fixture decorator (e.g., pytest_asyncio.fixture(...,
name="server") and name="graph_cleanup") and rename the underlying functions to
properly typed private names (e.g., _server and _graph_cleanup) with correct
async signatures so linters are happy; ensure the decorator retains scope and
autouse settings and that references to server/graph_cleanup continue to be
provided by the fixture name.

In `@autogpt_platform/backend/backend/copilot/sdk/tool_adapter.py`:
- Around line 393-395: The sandbox bridge uses the original file_path instead of
the canonical path read earlier (resolved), creating a TOCTOU mismatch; update
the call to _bridge_to_sandbox so it passes resolved (the canonical path
obtained at read time) instead of file_path and ensure the same resolved
variable (used when reading the file) is used for any sandbox copying in the
_current_sandbox branch (references: _current_sandbox, _bridge_to_sandbox,
resolved, file_path).
🪄 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: c7770d25-9e3e-4911-bc03-387f34415937

📥 Commits

Reviewing files that changed from the base of the PR and between f3dd708 and 263cd0e.

📒 Files selected for processing (6)
  • autogpt_platform/backend/backend/copilot/context.py
  • autogpt_platform/backend/backend/copilot/prompting.py
  • autogpt_platform/backend/backend/copilot/sdk/conftest.py
  • autogpt_platform/backend/backend/copilot/sdk/e2b_file_tools.py
  • autogpt_platform/backend/backend/copilot/sdk/e2b_file_tools_test.py
  • autogpt_platform/backend/backend/copilot/sdk/tool_adapter.py
✅ Files skipped from review due to trivial changes (1)
  • autogpt_platform/backend/backend/copilot/prompting.py
🚧 Files skipped from review as they are similar to previous changes (2)
  • autogpt_platform/backend/backend/copilot/context.py
  • autogpt_platform/backend/backend/copilot/sdk/e2b_file_tools.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: Seer Code Review
  • GitHub Check: end-to-end tests
  • GitHub Check: test (3.12)
  • GitHub Check: test (3.13)
  • GitHub Check: test (3.11)
  • GitHub Check: Analyze (python)
  • GitHub Check: Check PR Status
🧰 Additional context used
📓 Path-based instructions (4)
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

Refer to @backend/CLAUDE.md for backend-specific commands, architecture, and development tasks

autogpt_platform/backend/**/*.py: Import only at the top level; no local/inner imports except for lazy imports of heavy optional dependencies like openpyxl
Use absolute imports with from backend.module import ... for cross-package imports; single-dot relative imports (from .sibling import ...) are acceptable for sibling modules within the same package; avoid double-dot relative imports (from ..parent import ...)
Do not use duck typing with hasattr(), getattr(), or isinstance() for type dispatch; use typed interfaces, unions, or protocols instead
Use Pydantic models for structured data instead of dataclasses, namedtuples, or dicts
Do not use linter suppressors; no # type: ignore, # noqa, or # pyright: ignore comments — fix the underlying type/code issue instead
Use list comprehensions instead of manual loop-and-append patterns
Use early return guard clauses to avoid deep nesting
Use %s for deferred interpolation in debug log statements; use f-strings for readability in other log levels (e.g., logger.debug("Processing %s items", count), logger.info(f"Processing {count} items"))
Sanitize error paths using os.path.basename() in error messages to avoid leaking directory structure
Avoid TOCTOU (time-of-check-time-of-use) patterns; do not use check-then-act patterns for file access and credit charging operations
Use Redis pipelines with transaction=True for atomicity on multi-step Redis operations
Use max(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 h...

Files:

  • autogpt_platform/backend/backend/copilot/sdk/conftest.py
  • autogpt_platform/backend/backend/copilot/sdk/tool_adapter.py
  • autogpt_platform/backend/backend/copilot/sdk/e2b_file_tools_test.py
autogpt_platform/**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

Format Python code with poetry run format

Files:

  • autogpt_platform/backend/backend/copilot/sdk/conftest.py
  • autogpt_platform/backend/backend/copilot/sdk/tool_adapter.py
  • autogpt_platform/backend/backend/copilot/sdk/e2b_file_tools_test.py
autogpt_platform/backend/**/*test*.py

📄 CodeRabbit inference engine (AGENTS.md)

Run poetry run test for backend testing (runs pytest with docker based postgres + prisma)

Files:

  • autogpt_platform/backend/backend/copilot/sdk/conftest.py
  • autogpt_platform/backend/backend/copilot/sdk/e2b_file_tools_test.py
autogpt_platform/backend/**/*_test.py

📄 CodeRabbit inference engine (autogpt_platform/backend/CLAUDE.md)

autogpt_platform/backend/**/*_test.py: Use pytest with snapshot testing for API responses; test files should be colocated with source files using the *_test.py naming pattern
Mock at boundaries by mocking where the symbol is used, not where it is defined. After refactoring, update mock targets to match new module paths
Use AsyncMock from unittest.mock for mocking async functions

Files:

  • autogpt_platform/backend/backend/copilot/sdk/e2b_file_tools_test.py
🧠 Learnings (15)
📓 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:41.600Z
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: 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.
📚 Learning: 2026-03-25T06:59:27.340Z
Learnt from: CR
Repo: Significant-Gravitas/AutoGPT PR: 0
File: autogpt_platform/backend/CLAUDE.md:0-0
Timestamp: 2026-03-25T06:59:27.340Z
Learning: Applies to autogpt_platform/backend/**/*_test.py : Use `AsyncMock` from `unittest.mock` for mocking async functions

Applied to files:

  • autogpt_platform/backend/backend/copilot/sdk/conftest.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/conftest.py
📚 Learning: 2026-03-25T06:59:27.340Z
Learnt from: CR
Repo: Significant-Gravitas/AutoGPT PR: 0
File: autogpt_platform/backend/CLAUDE.md:0-0
Timestamp: 2026-03-25T06:59:27.340Z
Learning: Applies to autogpt_platform/backend/**/*_test.py : Mock at boundaries by mocking where the symbol is used, not where it is defined. After refactoring, update mock targets to match new module paths

Applied to files:

  • autogpt_platform/backend/backend/copilot/sdk/conftest.py
📚 Learning: 2026-03-25T06:59:27.340Z
Learnt from: CR
Repo: Significant-Gravitas/AutoGPT PR: 0
File: autogpt_platform/backend/CLAUDE.md:0-0
Timestamp: 2026-03-25T06:59:27.340Z
Learning: Applies to autogpt_platform/backend/**/*_test.py : Use pytest with snapshot testing for API responses; test files should be colocated with source files using the `*_test.py` naming pattern

Applied to files:

  • autogpt_platform/backend/backend/copilot/sdk/conftest.py
📚 Learning: 2026-03-25T06:58:59.806Z
Learnt from: CR
Repo: Significant-Gravitas/AutoGPT PR: 0
File: autogpt_platform/CLAUDE.md:0-0
Timestamp: 2026-03-25T06:58:59.806Z
Learning: Applies to autogpt_platform/backend/**/test_*.py,backend/**/*_test.py : Mark failing tests with pytest.mark.xfail in backend code during TDD before implementing the fix

Applied to files:

  • autogpt_platform/backend/backend/copilot/sdk/conftest.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/conftest.py
  • autogpt_platform/backend/backend/copilot/sdk/tool_adapter.py
📚 Learning: 2026-03-04T23:58:18.476Z
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.

Applied to files:

  • autogpt_platform/backend/backend/copilot/sdk/conftest.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/conftest.py
  • autogpt_platform/backend/backend/copilot/sdk/tool_adapter.py
  • autogpt_platform/backend/backend/copilot/sdk/e2b_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/sdk/conftest.py
  • autogpt_platform/backend/backend/copilot/sdk/tool_adapter.py
  • autogpt_platform/backend/backend/copilot/sdk/e2b_file_tools_test.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/sdk/conftest.py
  • autogpt_platform/backend/backend/copilot/sdk/tool_adapter.py
  • autogpt_platform/backend/backend/copilot/sdk/e2b_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/sdk/conftest.py
  • autogpt_platform/backend/backend/copilot/sdk/tool_adapter.py
  • autogpt_platform/backend/backend/copilot/sdk/e2b_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/sdk/conftest.py
  • autogpt_platform/backend/backend/copilot/sdk/tool_adapter.py
  • autogpt_platform/backend/backend/copilot/sdk/e2b_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/sdk/conftest.py
  • autogpt_platform/backend/backend/copilot/sdk/tool_adapter.py
  • autogpt_platform/backend/backend/copilot/sdk/e2b_file_tools_test.py
📚 Learning: 2026-04-01T04:17:35.195Z
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:35.195Z
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.py
  • autogpt_platform/backend/backend/copilot/sdk/e2b_file_tools_test.py
🔇 Additional comments (3)
autogpt_platform/backend/backend/copilot/sdk/e2b_file_tools_test.py (3)

16-23: Good test-surface expansion imports.

Importing _bridge_to_sandbox with its size thresholds keeps the new routing tests explicit and behavior-locked.


133-151: Nice coverage for tool-outputs host-read allowlist path.

This directly validates the new allowlist behavior and cleanup path for encoded project dirs.


367-463: Strong _bridge_to_sandbox branch coverage.

The suite covers skip conditions, best-effort failures, and size-tier routing clearly; this materially reduces regression risk for E2B bridging.

Comment thread autogpt_platform/backend/backend/copilot/sdk/conftest.py Outdated
Comment thread autogpt_platform/backend/backend/copilot/sdk/tool_adapter.py Outdated
The "Moving files between storages" section only had direction labels
("Sandbox → Persistent") with no tool examples. Model didn't know HOW
to copy. Now shows write_workspace_file(source_path=...) for upload and
read_workspace_file(save_to_path=...) for download.

@autogpt-pr-reviewer autogpt-pr-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

All 8 specialists have reported. Compiling the final verdict now.


PR #12646 — fix(backend/copilot): fix tool output file reading between E2B and host
Author: majdyz | SHA: f3dd708 | Files: context.py (+7/-4), context_test.py (+16/-1), prompting.py (+10/-5), e2b_file_tools.py (+40/-3), e2b_file_tools_test.py (+21/-3), security_hooks_test.py (+1/-1), base.py (+7/-2), base_test.py (+1/-1)

🎯 Verdict: REQUEST_CHANGES

What This PR Does

The copilot's file-reading pipeline had three compounding bugs: (1) some SDK versions save tool outputs to tool-outputs/ instead of tool-results/, but the allowlist only accepted the latter — so the copilot couldn't read those files; (2) the XML tag used path= which the model confused with local filesystem paths vs. workspace storage paths; (3) even when SDK files were readable via Read, bash_exec in the E2B sandbox couldn't access them because they lived on the host. This PR fixes all three: expands the allowlist, renames the attribute to workspace_path=, and adds a bridge that auto-copies host-side SDK files into the sandbox at /tmp/<filename>.

Specialist Findings

🛡️ Security ✅ — The allowlist expansion is well-constrained: os.path.realpath() resolves symlinks, a valid UUID segment is required, and directory names are checked against a hardcoded set (not a pattern). os.path.basename() is safe against path traversal. The workspace_path rename is a positive security change reducing prompt confusion. Two medium concerns:
⚠️ e2b_file_tools.py:329-330/tmp/{basename} collision: two tool-result files from different conversations sharing the same basename silently overwrite each other. An attacker-influenced filename could clobber a file the model relies on.
⚠️ e2b_file_tools.py:135-145 — The bridge pre-stages host files in the sandbox as a side effect of reads, weakening host↔sandbox isolation. Blast radius is limited by the allowlist, but this is a new attack surface for exfiltration via prompt injection + bash_exec.

🏗️ Architecture ⚠️ — Good extraction of _bridge_to_sandbox() as a standalone function with clear responsibility. The workspace_path rename pays down real tech debt. However:
⚠️ e2b_file_tools.py:141-145 + :335-336 — Double file read: _read_local reads the file, then _bridge_to_sandbox opens and reads it again. The already-read content should be passed through. (Cross-ref: Performance flagged this too.)
⚠️ e2b_file_tools.py:330prompting.py:217 — The sandbox destination /tmp/{basename} is coupled across two files with no shared constant. The prompt presents bridging as guaranteed ("automatically copies") but the code is best-effort — if bridging fails silently, bash_exec gets a confusing FileNotFoundError. (Cross-ref: Discussion notes Sentry bot flagged this exact issue with no author response.)
⚠️ context.py:185 — The tool-results/tool-outputs duality accommodates SDK version drift, but there's no documentation of which SDK versions use which name — a landmine for future maintainers.

Performance ⚠️ — The bridge adds measurable overhead per copilot file read:
⚠️ e2b_file_tools.py:335-336 — Synchronous open() + fh.read() inside an async def blocks the event loop. Should use asyncio.to_thread(). (Cross-ref: Architect flagged the double-read.)
⚠️ e2b_file_tools.py:336 — No file size cap before sandbox upload. fh.read() slurps the entire file; _sandbox_write then base64-encodes it (+33% memory), creating a shell command string. A pathological file could hit shell argument limits or OOM.
⚠️ No caching — re-reading the same file bridges it again every time. A simple set() of bridged basenames would prevent redundant sandbox writes.

🧪 Testing ⚠️ — The allowlist expansion has basic coverage, but the core new feature (_bridge_to_sandbox) has zero test coverage.
⚠️ Missing entirely: Tests for _bridge_to_sandbox — happy path, offset/limit guards, error swallowing, sandbox write failure.
⚠️ Missing: Integration test that _handle_read_file invokes the bridge when sandbox is active.
⚠️ Missing: tool-outputs path through security hooks (only docstring updated in security_hooks_test.py, no new test).
⚠️ Missing: Negative edge cases — "tool-output" (singular), case variations, near-miss strings.

📖 Quality ✅ — Clean, well-structured PR. Good docstring on _bridge_to_sandbox. Comments updated consistently. Naming follows codebase conventions.
⚠️ e2b_file_tools.py:326 — Magic number 2000 should be extracted to a named constant like _BRIDGE_MIN_LIMIT.
⚠️ base.py:93-100 — The f-string in _persist_and_summarize now spans 7 continuation lines — approaching readability limits.

📦 Product ✅ — Core fix directly addresses user-visible pain: copilot failing to read tool outputs from certain SDK versions. The updated prompts in prompting.py are significantly clearer, separating SDK files from workspace storage. The bridge enables a new capability (processing SDK files with bash).
⚠️ No user-visible signal that bridging happened — if it fails silently, the model still tells the user the file is at /tmp/<file> but it won't be there.
⚠️ /tmp/{basename} collision could cause data loss when processing multiple tool outputs.

📬 Discussion ⚠️ — CI partially pending (3 Python test matrix jobs + e2e tests still running at time of review). No human approvals yet (REVIEW_REQUIRED).
⚠️ Sentry bot flagged e2b_file_tools.py:370 — silent exception swallowing in _bridge_to_sandbox with DEBUG-level logging. No response from author.
⚠️ CodeRabbit flagged base.py:100-102 — minor wording inconsistency between comment and output. No response from author.
⚠️ Author noted they couldn't run context_test.py and e2b_file_tools_test.py locally due to a pre-existing DB migration issue — relying on CI.

🔎 QA ✅ — Full live testing completed. Frontend loads, signup flow works, copilot UI accessible with chat interface. Copilot backend processed a message end-to-end (70+ seconds of streaming status updates). Build page and library page load correctly. No application-level console errors — only dev-mode noise (LaunchDarkly retries, CSS preload warnings).
landing
copilot-page
copilot-chat-response
build-page

Blockers (Must Fix)

  1. e2b_file_tools.py:315-349_bridge_to_sandbox() has zero test coverage. This is the core new behavior in the PR. Needs at minimum: happy-path test (offset=0, limit≥2000 → file bridged), guard tests (offset≠0 or limit<2000 → no bridge), and error-swallowing test (read/write failure → logged, not raised). (Flagged by: Testing, Security, Architect)

  2. e2b_file_tools.py:329-330/tmp/{basename} filename collision. Two tool-result files from different conversations with the same basename silently overwrite each other. Namespace with the conversation UUID: /tmp/{uuid_prefix}_{basename} or /tmp/tool-results/{basename}. (Flagged by: Security, Architect, Performance, Product — 4/8 specialists)

Should Fix (Follow-up OK)

  1. e2b_file_tools.py:335-336 — Synchronous open() in async context. Blocks the event loop. Use asyncio.to_thread() for the file read. (Performance)
  2. e2b_file_tools.py:141-145 — Double file read. Pass the already-read content from _read_local to _bridge_to_sandbox instead of re-reading from disk. (Architect, Performance)
  3. e2b_file_tools.py:336 — No file size cap. Add an explicit MAX_BRIDGE_SIZE (e.g. 512KB) and skip bridging for oversized files. (Performance)
  4. e2b_file_tools.py:340-345 — Promote bridge failure logging from DEBUG to WARNING. Silent DEBUG-level logging makes persistent failures invisible in production. (Security, Discussion/Sentry bot)
  5. e2b_file_tools.py:326 — Extract magic number 2000 to a named constant. (Quality)
  6. security_hooks_test.py — Add test_read_tool_outputs_allowed. The existing test only covers tool-results. (Testing)

Risk Assessment

Merge risk: MEDIUM | Rollback: EASY (revert one commit, no migrations)

The core logic change (allowlist expansion + workspace_path rename) is clean and low-risk. The bridge feature introduces new side effects on the read path with no test coverage and a filename collision bug — these should be addressed before merge. The fix itself is clearly needed and the approach is sound; the issues are in the implementation details.

REVIEW_COMPLETE
PR: #12646
Verdict: REQUEST_CHANGES
Blockers: 2

… None

- Pass `resolved` (realpath-expanded) to `_bridge_to_sandbox` in
  `_read_file_handler` so the bridge target matches the file that was
  actually read (addresses review comment).
- Replace bare `return` with explicit `return None` in
  `_bridge_to_sandbox` large-file skip path for consistency with the
  declared `str | None` return type.
@github-project-automation github-project-automation Bot moved this from 🆕 Needs initial review to 🚧 Needs work in AutoGPT development kanban Apr 2, 2026
majdyz added 2 commits April 2, 2026 08:26
…ardcoded /home/user

The storage supplement template and _persist_and_summarize had hardcoded
/home/user/ paths in save_to_path examples. In local (bubblewrap) mode
the working dir is /tmp/copilot-<session>/, not /home/user/. Use the
{working_dir} template variable in prompting.py and a generic
<working_dir> placeholder in base.py so the model gets correct paths
regardless of execution mode.
…xtures

Address CodeRabbit review: remove # type: ignore[override] from SDK
conftest fixtures per AGENTS.md no-suppressor rule. Use name= parameter
in pytest_asyncio.fixture decorator with private function names instead.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (1)
autogpt_platform/backend/backend/copilot/sdk/e2b_file_tools.py (1)

316-381: Extract the bridge logic before this file grows further.

This file is already over 500 lines, and _bridge_to_sandbox() adds another 50+ line responsibility on top of the MCP handlers. Splitting the host→sandbox bridge strategy into a dedicated helper/module will keep the read path easier to follow and test.

As per coding guidelines, "Keep files under ~300 lines; if a file grows beyond this, split by responsibility (extract helpers, models, or create a sub-module)." and "Keep functions under ~40 lines; extract named helpers when a function grows longer. Long functions indicate mixed concerns, not complexity."

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

In `@autogpt_platform/backend/backend/copilot/sdk/e2b_file_tools.py` around lines
316 - 381, Extract the host→sandbox bridge into a dedicated helper module: move
the function _bridge_to_sandbox plus related constants (_BRIDGE_SHELL_MAX_BYTES,
_BRIDGE_SKIP_BYTES) and any helpers it calls (_sandbox_write usage) into a new
module (e.g., copilot/sdk/bridge.py or similar), leaving a thin
import-forwarding call in e2b_file_tools.py; ensure you preserve exact behavior
for size thresholds, file expansion, error logging, and calls to
sandbox.files.write, update imports in e2b_file_tools.py to use the new module,
and add a small unit test for the new bridge module to cover the three size
branches and the error/logging path.
🤖 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/e2b_file_tools.py`:
- Around line 348-369: The code currently uses basename to build sandbox_path
(variables basename, expanded, sandbox_path) which collapses different source
files with the same name; change the target path derivation to use the resolved
expanded path (e.g., include a safe, collision-free representation of expanded
such as a sanitized path fragment or a hash of expanded) when creating
sandbox_path before calling _sandbox_write or sandbox.files.write so each source
maps to a unique sandbox location; ensure the same transformation is applied in
both branches (the _sandbox_write call and sandbox.files.write call) so
sandbox_path is consistent and collision-free.
- Around line 321-370: The _bridge_to_sandbox function decodes file bytes with
errors="replace" and uses a too-large _BRIDGE_SHELL_MAX_BYTES, which corrupts
binary/non-UTF8 files and can exceed shell ARG_MAX when using _sandbox_write;
also basename collisions risk overwriting different files. Fix by preserving
bytes (do not .decode() for binary transfers), use sandbox.files.write in binary
mode for all sizes that may contain non-UTF8 content, lower the shell-transfer
cutoff _BRIDGE_SHELL_MAX_BYTES to a safe small value (e.g., 8–16 KB) so
_sandbox_write is only used for tiny text files, and create sandbox_path using a
unique identifier (e.g., include a short hash or parent directory fragment
alongside basename) so different host files don’t collide; modify
_bridge_to_sandbox, _BRIDGE_SHELL_MAX_BYTES, _BRIDGE_SKIP_BYTES, _sandbox_write
usage and sandbox.files.write calls accordingly.

---

Nitpick comments:
In `@autogpt_platform/backend/backend/copilot/sdk/e2b_file_tools.py`:
- Around line 316-381: Extract the host→sandbox bridge into a dedicated helper
module: move the function _bridge_to_sandbox plus related constants
(_BRIDGE_SHELL_MAX_BYTES, _BRIDGE_SKIP_BYTES) and any helpers it calls
(_sandbox_write usage) into a new module (e.g., copilot/sdk/bridge.py or
similar), leaving a thin import-forwarding call in e2b_file_tools.py; ensure you
preserve exact behavior for size thresholds, file expansion, error logging, and
calls to sandbox.files.write, update imports in e2b_file_tools.py to use the new
module, and add a small unit test for the new bridge module to cover the three
size branches and the error/logging path.
🪄 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: f206b527-9f0d-41a7-8917-031082cc48a6

📥 Commits

Reviewing files that changed from the base of the PR and between 0e567df and 3a49086.

📒 Files selected for processing (2)
  • autogpt_platform/backend/backend/copilot/sdk/e2b_file_tools.py
  • autogpt_platform/backend/backend/copilot/sdk/tool_adapter.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: end-to-end tests
  • GitHub Check: test (3.12)
  • GitHub Check: test (3.11)
  • GitHub Check: type-check (3.11)
  • GitHub Check: test (3.13)
  • GitHub Check: Analyze (python)
  • GitHub Check: Check PR Status
  • GitHub Check: Seer Code Review
🧰 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

Refer to @backend/CLAUDE.md for backend-specific commands, architecture, and development tasks

autogpt_platform/backend/**/*.py: Import only at the top level; no local/inner imports except for lazy imports of heavy optional dependencies like openpyxl
Use absolute imports with from backend.module import ... for cross-package imports; single-dot relative imports (from .sibling import ...) are acceptable for sibling modules within the same package; avoid double-dot relative imports (from ..parent import ...)
Do not use duck typing with hasattr(), getattr(), or isinstance() for type dispatch; use typed interfaces, unions, or protocols instead
Use Pydantic models for structured data instead of dataclasses, namedtuples, or dicts
Do not use linter suppressors; no # type: ignore, # noqa, or # pyright: ignore comments — fix the underlying type/code issue instead
Use list comprehensions instead of manual loop-and-append patterns
Use early return guard clauses to avoid deep nesting
Use %s for deferred interpolation in debug log statements; use f-strings for readability in other log levels (e.g., logger.debug("Processing %s items", count), logger.info(f"Processing {count} items"))
Sanitize error paths using os.path.basename() in error messages to avoid leaking directory structure
Avoid TOCTOU (time-of-check-time-of-use) patterns; do not use check-then-act patterns for file access and credit charging operations
Use Redis pipelines with transaction=True for atomicity on multi-step Redis operations
Use max(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 h...

Files:

  • autogpt_platform/backend/backend/copilot/sdk/e2b_file_tools.py
autogpt_platform/**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

Format Python code with poetry run format

Files:

  • autogpt_platform/backend/backend/copilot/sdk/e2b_file_tools.py
🧠 Learnings (13)
📓 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:41.600Z
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: 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: 12356
File: autogpt_platform/backend/backend/copilot/constants.py:9-12
Timestamp: 2026-03-10T08:39:22.025Z
Learning: In Significant-Gravitas/AutoGPT PR `#12356`, the `COPILOT_SYNTHETIC_ID_PREFIX = "copilot-"` check in `create_auto_approval_record` (human_review.py) is intentional and safe. The `graph_exec_id` passed to this function comes from server-side `PendingHumanReview` DB records (not from user input); the API only accepts `node_exec_id` from users. Synthetic `copilot-*` IDs are only ever created server-side in `run_block.py`. The prefix skip avoids a DB lookup for a `AgentGraphExecution` record that legitimately does not exist for CoPilot sessions, while `user_id` scoping is enforced at the auth layer and on the resulting auto-approval record.
📚 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/e2b_file_tools.py
📚 Learning: 2026-04-01T04:17:35.195Z
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:35.195Z
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/e2b_file_tools.py
📚 Learning: 2026-03-10T08:39:22.025Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12356
File: autogpt_platform/backend/backend/copilot/constants.py:9-12
Timestamp: 2026-03-10T08:39:22.025Z
Learning: In Significant-Gravitas/AutoGPT PR `#12356`, the `COPILOT_SYNTHETIC_ID_PREFIX = "copilot-"` check in `create_auto_approval_record` (human_review.py) is intentional and safe. The `graph_exec_id` passed to this function comes from server-side `PendingHumanReview` DB records (not from user input); the API only accepts `node_exec_id` from users. Synthetic `copilot-*` IDs are only ever created server-side in `run_block.py`. The prefix skip avoids a DB lookup for a `AgentGraphExecution` record that legitimately does not exist for CoPilot sessions, while `user_id` scoping is enforced at the auth layer and on the resulting auto-approval record.

Applied to files:

  • autogpt_platform/backend/backend/copilot/sdk/e2b_file_tools.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/sdk/e2b_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/e2b_file_tools.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/sdk/e2b_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/sdk/e2b_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/e2b_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/e2b_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/e2b_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/e2b_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/e2b_file_tools.py

Comment thread autogpt_platform/backend/backend/copilot/sdk/e2b_file_tools.py Outdated
Comment thread autogpt_platform/backend/backend/copilot/sdk/e2b_file_tools.py Outdated
…dbox paths

- Lower _BRIDGE_SHELL_MAX_BYTES from 5 MB to 32 KB to stay within
  ARG_MAX when base64-encoding content for shell transfer.
- Prefix bridged sandbox filenames with a 12-char SHA-256 hash of the
  full source path to prevent collisions when different source files
  share the same basename (e.g. multiple result.json files).
- Fix potential NameError in exception handler when basename is not yet
  assigned.
@majdyz

majdyz commented Apr 2, 2026

Copy link
Copy Markdown
Contributor Author

/review

@autogpt-pr-reviewer

Copy link
Copy Markdown

Queued a review for PR #12646 at dd34b0d.

@majdyz

majdyz commented Apr 2, 2026

Copy link
Copy Markdown
Contributor Author

E2E Test Report - PR #12646 (Round 1)

Test date: 2026-04-02
Server: localhost:3000/8006, combined-preview-test
User: test@test.com
Branch: fix/copilot-tool-output-e2b-bridging


1. Code Review Summary

22 files changed across 9 commits. Changes fall into these categories:

A. Core Feature: E2B File Bridging (main PR scope)

  • e2b_file_tools.py: New _bridge_to_sandbox() function copies SDK-internal tool-result files into the E2B sandbox so bash_exec can access them. Size-tiered routing: <=32KB via shell base64 to /tmp/, 32KB-50MB via sandbox.files.write() to /home/user/, >50MB skipped. Collision-free names via SHA256 prefix.
  • e2b_file_tools.py _handle_read_file(): Now calls _bridge_to_sandbox() after reading local SDK files, appending [Sandbox copy available at ...] to the result.
  • tool_adapter.py _read_file_handler(): Same bridging behavior for the Read MCP tool (handles workspace:// URIs and local paths).
  • prompting.py: Added "SDK tool-result files in E2B" section explaining auto-bridging. Added "Handling binary/image data" and "Passing large data between tools" CRITICAL sections. Added concrete write_workspace_file/read_workspace_file examples with {working_dir} template instead of hardcoded /home/user.

B. Cleanup & Reverts (bundled)

  • service.py: Reverted prompt-too-long detection in AssistantMessage content (now only checks error_text, not error_preview to avoid false positives). Reverted ResultMessage prompt-too-long check from "always" to "only on error subtype". Removed compaction text-end pre-close logic.
  • creds_manager.py: Simplified refresh_if_needed() — removed _refresh_locked/_refresh_unlocked split, merged into single method with conditional lock. Removed _get_oauth_handler helper. Manager now owns its own _locks instead of delegating to store.
  • credentials_store.py: Replaced @thread_cached locks with simple _locks instance cache.
  • integration_creds.py: Minor simplification.
  • blocks/io.py: Reverted AgentDropdownInputBlockoptions field back to placeholder_values, removed AliasChoices backward-compat logic.
  • Test deletions: Removed tests for reverted features (prompt-too-long content detection, thread-safe locks, compaction text-end, dropdown backward compat).

C. Code Observations

  • _BRIDGE_SHELL_MAX_BYTES = 32 * 1024 — this is lower than the _E2B_TOOL_NOTES prompt which says "5 MB" threshold. The prompt says "/tmp/ (or /home/user/ for files >5 MB)" but the actual code threshold is 32 KB. Minor doc mismatch.
  • _bridge_to_sandbox reads the full file into memory with open(expanded, "rb") then decodes as UTF-8. For binary files like images, this would still work (errors="replace") but the content would be garbled. This is fine since bridging is for tool-result JSON files, not binary data.

2. Unit Tests

All relevant test suites pass:

Test Suite Result
e2b_file_tools_test.py 46/46 passed
context_test.py 29/29 passed
base_test.py 9/9 passed
integration_creds_test.py 14/14 passed
security_hooks_test.py 28/28 passed
Total 126/126 passed

3. E2E Browser Test

Prompt: "Create a simple Python script that generates a bar chart showing quarterly sales data and save it as chart.png. Show me the chart when done."

CoPilot Processing

The CoPilot used the E2B sandbox to execute the full pipeline:

CoPilot processing with tool calls

Chart Result Displayed

The chart was generated in the E2B sandbox, bridged to workspace storage, and displayed inline:

Chart result in chat

Full Chat Session

Full chat session


4. Executor Logs Confirm E2B Bridging

SDK file bridging (the core new feature) was exercised:

[E2B] Bridged SDK file to sandbox: toolu_bdrk_01QB8WCanZTYZXAzsEwQWkx8.json -> /tmp/toolu_bdrk_01QB8WCanZTYZXAzsEwQWkx8.json

Full tool flow for chart session:

StreamToolInputAvailable, tool=bash_exec          # install matplotlib
StreamToolOutputAvailable, tool=bash_exec          # success
StreamToolInputAvailable, tool=write_file          # write chart.py to sandbox
StreamToolOutputAvailable, tool=write_file          # success (41 chars)
StreamToolInputAvailable, tool=bash_exec          # python chart.py
StreamToolOutputAvailable, tool=bash_exec          # success (190 chars)
StreamToolInputAvailable, tool=write_workspace_file # bridge chart.png
Wrote file b4afb21e (chart.png) size=50021 bytes  # 50KB chart saved
StreamToolOutputAvailable, tool=write_workspace_file # success (529 chars)

E2B sandbox lifecycle: Created -> Used -> Paused -> Reconnected -> Used -> Paused


5. Issues Found

# Severity Description
1 Low Prompt says bridging threshold is "5 MB" but code uses 32 KB (_BRIDGE_SHELL_MAX_BYTES). The 32 KB is the shell vs files.write routing threshold, not the skip threshold (50 MB). The prompt wording is misleading.
2 Info The blocks/io.py changes (reverting options -> placeholder_values) and creds_manager.py simplification are out-of-scope for the PR title "tool output E2B bridging". These are cleanup/reverts from prior PRs. Consider splitting into separate PRs for cleaner review.
3 Info Deleted tests (response_adapter_test, retry_scenarios_test) were for features reverted in the same PR (prompt-too-long content detection, compaction text-end). Deletions are justified since the features were reverted.

Verdict

E2B file bridging works correctly. The core feature (copying SDK tool-result files into the E2B sandbox) is functional and well-tested with 46 dedicated unit tests. The E2E test confirmed the full pipeline: write_file -> bash_exec -> write_workspace_file with the chart displayed inline in the chat.

The bundled cleanup changes (creds_manager simplification, blocks/io revert, prompt-too-long revert) are logically separate from the bridging feature but don't introduce regressions.

Comment thread autogpt_platform/backend/backend/copilot/sdk/e2b_file_tools.py Outdated
_bridge_to_sandbox was decoding all file content with
`errors='replace'`, silently corrupting non-UTF-8 bytes (images, PDFs,
etc.) by replacing them with U+FFFD.

Now attempts strict UTF-8 decode first; on failure writes raw bytes
via sandbox.files.write() (which accepts Union[str, bytes, IO]) or
base64-encoded shell pipe for /tmp paths.

Also updates _sandbox_write to accept str | bytes and adds tests for
both small and large binary file bridging.
@majdyz

majdyz commented Apr 2, 2026

Copy link
Copy Markdown
Contributor Author

Fixed in dd228de. _bridge_to_sandbox now attempts strict UTF-8 decode first; on failure writes raw bytes via sandbox.files.write() (which accepts Union[str, bytes, IO]) or base64-encoded shell pipe for /tmp paths. Also updated _sandbox_write to accept str | bytes and added tests for both small and large binary file bridging.

@autogpt-pr-reviewer

Copy link
Copy Markdown

All 8 specialists have reported. Compiling the final verdict now.


PR #12646 — fix(backend/copilot): fix tool output file reading between E2B and host
Author: majdyz | SHA: dd34b0d | Files: context.py (+6/-3), context_test.py (+16/-1), prompting.py (+25/-6), conftest.py (+16), e2b_file_tools.py (+83/-2), e2b_file_tools_test.py (+129/-3), security_hooks_test.py (+1/-1), tool_adapter.py (+11/-2), base.py (+8/-3), base_test.py (+1/-1)

🎯 Verdict: APPROVE

What This PR Does

The copilot's Claude SDK sometimes saves tool outputs to tool-outputs/ instead of tool-results/, and files read from the host filesystem couldn't be accessed by bash_exec running inside E2B sandboxes. This PR fixes three compounding issues: (1) adds tool-outputs/ to the allowed-path allowlist alongside tool-results/, (2) auto-bridges SDK-internal files into the E2B sandbox so bash can process them, and (3) renames the truncation tag attribute from path to workspace_path to prevent the model from confusing workspace paths with filesystem paths. Updated prompts guide the model on correct file handling patterns.

Specialist Findings

🛡️ Security ✅ — Allowlist widening is safe: is_allowed_local_path() adds one string literal ("tool-outputs") to an exact-match tuple check. All existing guards remain intact — os.path.realpath() resolves symlinks, UUID regex validates the conversation directory, and project_dir is verified against SDK_PROJECTS_DIR. The _bridge_to_sandbox function only receives paths already validated by callers. No path traversal, no exfiltration vector. The pathworkspace_path rename is a positive security improvement reducing model confusion.
⚠️ e2b_file_tools.py:327_bridge_to_sandbox has no internal allowlist check. Currently safe because both callers validate first, but a future caller could forget. Consider adding defence-in-depth validation inside the function.

🏗️ Architecture ⚠️ — The bridge-and-append pattern is duplicated in two places: e2b_file_tools.py:141-148 and tool_adapter.py:393-400. Both do identical _bridge_to_sandbox + append [Sandbox copy available at ...]. A helper like _maybe_bridge_and_annotate() would collapse both to one line. The size-tiering (shell ≤32KB, API 32KB–50MB, skip >50MB) is well-reasoned with clear constants and documentation.
⚠️ tool_adapter.py:41_bridge_to_sandbox is imported cross-module despite the _ private prefix. Should be renamed to bridge_to_sandbox or extracted to a shared module. Both Architect and Quality flagged this independently — strong signal.
⚠️ prompting.py carries implementation details (specific byte thresholds) in system prompts. Pragmatic but creates implicit coupling between prompt text and code constants.

Performance ⚠️ — Bridging runs inline on every qualifying Read call with no caching. Repeated reads of the same file re-read from disk and re-transfer to sandbox unnecessarily. For files near the 50MB limit, fh.read() + .decode() creates ~100-200MB transient memory (bytes + str copies). The content.decode("utf-8", errors="replace") silently corrupts binary files (images, PDFs) by replacing invalid bytes with U+FFFD. Performance and Security both note this.
⚠️ e2b_file_tools.py:369 — Consider a session-scoped set of already-bridged paths to skip redundant transfers.
⚠️ e2b_file_tools.py:372-377 — Binary file corruption via errors="replace". Document that bridging is text-only, or pass raw bytes for the files.write() path.

🧪 Testing ⚠️TestBridgeToSandbox class provides excellent unit coverage (7 scenarios: happy path, skip conditions, errors, size tiers). tool-outputs path validation tested. However, the tool_adapter.py bridge call site has zero test coverage — only the e2b_file_tools.py call site is tested directly. No integration test verifies the [Sandbox copy available...] message appears in the MCP result from either call path.
⚠️ tool_adapter.py:393-397 — Missing integration test for the second bridge call site (uses _current_sandbox.get() + resolved path, different from the other site).
⚠️ e2b_file_tools.py:345 — Magic number 2000 for the limit threshold has no named constant or explanation. Boundary value limit=1999 not tested.

📖 Quality ✅ — Code is well-documented. _bridge_to_sandbox has an excellent docstring covering purpose, skip conditions, size tiers, and naming strategy. Constants have clear inline comments explaining rationale (ARG_MAX, transfer times). Test docstrings all explain what they verify. The workspace_path rename is consistent across all files.
⚠️ e2b_file_tools.py module docstring (line 7) still says only tool-results/ — should mention tool-outputs/ too.
⚠️ .decode("utf-8", errors="replace") appears in both branches — could be deduplicated above the if/else.

📦 Product ✅ — All three user-facing issues addressed: tool-outputs/ allowed, files bridged to sandbox, path attribute disambiguated. No breaking changes to existing sessions. New prompt sections are well-structured and follow existing patterns.
⚠️ Prompt/code threshold mismatchprompting.py:155 says files >5 MB go to /home/user/ but e2b_file_tools.py:320 uses _BRIDGE_SHELL_MAX_BYTES = 32 * 1024 (32 KB). The model may look in /tmp/ for a 1 MB file that's actually in /home/user/. This will cause confusion. Should fix.

📬 Discussion ✅ — 18/22 CI checks passing, 4 pending/skipped (expected). No human reviewers yet. Author addressed all major bot concerns across 5 follow-up commits: TOCTOU fix, basename collision fix, shell threshold lowered from 5MB to 32KB, conftest fixture fixes. Two unaddressed Sentry concerns: (1) silent error swallowing in bridge at DEBUG level, (2) binary file corruption via errors="replace". The CHANGES_REQUESTED from autogpt-pr-reviewer is stale — both original blockers were fixed in subsequent commits.

🔎 QA ✅ — Full live testing performed. Landing page, signup, login, copilot chat, and build pages all functional. Copilot accepts and processes messages without errors. Health endpoint returns 200. WebSocket connected. No console errors related to the changed code. No regressions detected.

landing
copilot-page
copilot-response
build-after-login

Blockers (Must Fix)

None.

Should Fix (Follow-up OK)

  1. prompting.py:155 — Prompt says "5 MB" threshold but code uses 32 KB (e2b_file_tools.py:320). Model will look for bridged files in the wrong location. Fix prompt to say "~32 KB" or align the constant. (Flagged by Product; author's own E2E report also noted this.)
  2. tool_adapter.py:393-397 — Missing test coverage for second bridge call site. The e2b_file_tools.py path is well-tested but the tool_adapter.py integration is untested. (Flagged by Testing.)
  3. e2b_file_tools.py:372-377 — Binary file silent corruption. .decode("utf-8", errors="replace") corrupts non-UTF-8 content. Document text-only assumption or pass raw bytes for the files.write() path. (Cross-referenced: Performance, Testing, Discussion/Sentry.)
  4. tool_adapter.py:41 / e2b_file_tools.py:327 — Private function exported cross-module. Rename _bridge_to_sandboxbridge_to_sandbox or add defence-in-depth allowlist check inside the function. (Cross-referenced: Security, Architect, Quality.)
  5. e2b_file_tools.py:141-148 + tool_adapter.py:393-400 — Duplicated bridge-and-append pattern. Extract to a shared helper. (Cross-referenced: Architect, Quality.)

Risk Assessment

Merge risk: LOW | Rollback: EASY

The changes are additive (new allowlist entry, new bridge function, updated prompts). No database migrations, no API contract changes, no breaking changes to existing sessions. The tool-outputs/ addition is a strict superset. Bridge failures are best-effort (caught, logged, never propagated). Rollback is a simple revert.

REVIEW_COMPLETE
PR: #12646
Verdict: APPROVE
Blockers: 0

- Use asyncio.to_thread for synchronous file read in async context
- Promote bridge failure logging from DEBUG to WARNING
- Extract magic number 2000 to _DEFAULT_READ_LIMIT named constant
@majdyz

majdyz commented Apr 2, 2026

Copy link
Copy Markdown
Contributor Author

Test Results: Read Tool Bridging Verification (PR #12646)

Test 1: Read tool bridging code review — PASS

The _handle_read_file function in e2b_file_tools.py (lines 132-172) correctly implements two read paths:

  1. SDK-internal paths (tool-results, ephemeral CWD): Reads from host via _read_local(), then calls _bridge_to_sandbox() to copy the file into the E2B sandbox. The response is annotated with [Sandbox copy available at <path>].
  2. Sandbox paths (/home/user, /tmp): Reads directly from the sandbox via sandbox.files.read().

The bridging logic (_bridge_to_sandbox, lines 332-395) is well-structured:

  • Only bridges when offset=0 and limit >= 2000 (full-file reads)
  • Uses SHA-256 prefix on filenames to avoid collisions between files with the same basename
  • Handles both text (UTF-8) and binary files correctly
  • Errors are caught and logged but never propagated (best-effort)

Test 2: CoPilot end-to-end write+read — BLOCKED

Both test sessions failed with authentication_failed / API Error: 401 {"error":{"message":"User not found.","code":401}}. This is an Anthropic API authentication issue in the test environment, not related to the PR code. The copilot executor starts, creates an E2B sandbox, but fails when calling the Claude SDK.

Test 3: Executor logs for Read bridging — PASS (from prior session)

Found evidence of successful bridging from a prior working session:

[SDK][7e90aa69][T1] Tool event: StreamToolInputAvailable, tool=Read
[E2B] Bridged SDK file to sandbox: toolu_bdrk_01QB8WCanZTYZXAzsEwQWkx8.json -> /tmp/toolu_bdrk_01QB8WCanZTYZXAzsEwQWkx8.json
[SDK] PostToolUse: mcp__copilot__Read (builtin=False)

The flow was: find_library_agentread_workspace_fileRead (bridged to /tmp/) → bash_exec (could access bridged file). This confirms the Read tool bridging works end-to-end in a real copilot session.

Test 4: {working_dir} template — PASS (with minor docs issue)

prompting.py correctly uses {working_dir} as a template parameter in _build_storage_supplement():

  • Local mode: called with cwd (dynamic)
  • E2B mode: called with "/home/user" (fixed E2B workdir)

No hardcoded /home/user in path logic. The references on lines 46, 92, 120 are only in example @@agptfile: strings.

Minor docs mismatch: Line 155 of prompting.py says files >5 MB go to /home/user/<filename>, but the actual code threshold is 32 KB (_BRIDGE_SHELL_MAX_BYTES). The 5 MB figure appears to be stale. This is cosmetic — it's LLM prompt guidance, not logic — but could be corrected.

Test 5: Size limit verification — PASS

Two-tier size handling is correctly implemented:

Range Location Method
<= 32 KB /tmp/<hash>-<basename> Shell base64 via _sandbox_write()
32 KB - 50 MB /home/user/<hash>-<basename> sandbox.files.write() (avoids ARG_MAX)
> 50 MB Skipped Warning logged

The 32 KB shell threshold stays well within Linux ARG_MAX (128 KB) even after ~33% base64 expansion. The 50 MB skip threshold prevents excessive transfer times.

Summary

Test Status
Read bridging code review PASS
CoPilot E2E write+read BLOCKED (API auth — unrelated)
Executor logs for bridging PASS (from prior session)
{working_dir} template PASS (minor docs mismatch: prompt says 5 MB, code uses 32 KB)
Size limits PASS

Recommendation: The Read tool bridging implementation is solid. Consider updating the prompt text on line 155 of prompting.py from ">5 MB" to ">32 KB" to match the actual _BRIDGE_SHELL_MAX_BYTES threshold.

@majdyz

majdyz commented Apr 2, 2026

Copy link
Copy Markdown
Contributor Author

Retest: 3 new commits (dd228de, dd34b0d, 19ea753)

Test results

All 48 tests passed in e2b_file_tools_test.py (was 42 before these commits — 6 new tests added).

Code review of changes

1. Binary file preservation (dd228de) — Correct

  • _sandbox_write now accepts str | bytes instead of just str. The base64 pipeline handles both: content.encode() for str, raw passthrough for bytes.
  • _bridge_to_sandbox reads files as rb, then tries strict raw_content.decode("utf-8"). On UnicodeDecodeError, the raw bytes are kept as-is. This replaces the previous decode("utf-8", errors="replace") which silently garbled binary content with replacement characters.
  • Two new tests: test_small_binary_file_preserves_bytes (base64 path via /tmp) and test_large_binary_file_writes_raw_bytes (files.write path via /home/user), both using bytes(range(256)) as test data.

2. Lower bridge shell threshold + collision-free paths (dd34b0d) — Correct

  • Shell threshold lowered from 5 MB to 32 KB (_BRIDGE_SHELL_MAX_BYTES = 32 * 1024). Good rationale in the comment: base64 expands by ~33%, so 32 KB stays well under the typical Linux ARG_MAX of 128 KB.
  • Collision-free paths: sandbox filenames now use <sha256[:12]>-<basename> format. hashlib.sha256(expanded.encode()).hexdigest()[:12] produces a 12-char hex prefix from the full resolved source path. This prevents collisions when bridging multiple files with the same basename (e.g. different result.json files).
  • basename extraction moved after os.path.realpath() so it operates on the resolved path.
  • Error logging now includes the original file_path instead of just basename — better for debugging.
  • Test helper _expected_bridge_path mirrors the hash logic for assertions.

3. Review feedback (19ea753) — Correct

  • Extracted _DEFAULT_READ_LIMIT = 2000 constant, used in both _handle_read_file and the _bridge_to_sandbox threshold check. Eliminates the magic number.
  • File I/O moved to asyncio.to_thread(_read_bytes) so the blocking open()/read() doesn't stall the event loop.
  • Bridge failure logging upgraded from logger.debug to logger.warning — failures are worth seeing in production logs.
  • All tests updated to use _DEFAULT_READ_LIMIT instead of hardcoded 2000.

Summary

All three commits look correct. Binary preservation avoids the previous silent data corruption from errors="replace". The 32 KB shell threshold is well-justified. SHA-256 path prefix prevents filename collisions cleanly. Review feedback items (constant extraction, async I/O, log level) are all properly addressed.

@majdyz
majdyz requested review from Bentlybro, Pwuts and ntindle April 2, 2026 10:14
majdyz added 3 commits April 2, 2026 14:35
… allowlist

The allowlist was expanded to accept tool-outputs/ in addition to
tool-results/, but security_hooks_test.py only verified tool-results.
Add test_read_tool_outputs_allowed to close the security test coverage gap.
… prompt

The prompt said files >5 MB go to /home/user/ but the actual threshold
was lowered to 32 KB. Replace with a generic description that avoids
hardcoding the threshold and directs the model to the [Sandbox copy
available at ...] annotation instead.
@majdyz

majdyz commented Apr 2, 2026

Copy link
Copy Markdown
Contributor Author

/dev-review

1 similar comment
@majdyz

majdyz commented Apr 2, 2026

Copy link
Copy Markdown
Contributor Author

/dev-review

@autogpt-pr-reviewer-in-dev

Copy link
Copy Markdown

Queued a review for PR #12646 at 0b4acd7.

majdyz added 2 commits April 2, 2026 18:16
…face

- Rename _bridge_to_sandbox to bridge_to_sandbox (public) since it is
  imported cross-module from tool_adapter.py (item 4)
- Extract duplicated bridge+append-annotation pattern into shared
  bridge_and_annotate() helper used by both e2b_file_tools and
  tool_adapter (item 5)
- Add tests verifying bridge_and_annotate is called from
  _read_file_handler in tool_adapter when a sandbox is active (item 2)
- Add unit tests for bridge_and_annotate helper itself
@majdyz

majdyz commented Apr 2, 2026

Copy link
Copy Markdown
Contributor Author

🔴 Automated Review — Request Changes

Generated by pr-review-automation using Claude Max subscription (no API key) · 320s

Risk: 🟡 Medium

Summary

The three root-cause fixes (attribute rename, allowlist extension, sandbox bridging) are correct and well-motivated. However, the new bridging feature has duplicate call sites across two read handlers with no test confirming mutual exclusivity, weak test assertions that don't verify data fidelity through the bridge paths, and an exported function that reads arbitrary host files without internal path validation. Six specific changes are requested before merge.

Specialist Reports

Security — (9 findings)
  • 🔴 CRITICAL: None identified.
  • 🔵 LOW: 12-character SHA-256 path prefix for sandbox filename collision avoidance (e2b_file_tools.py line ~360): `hashlib.
  • 🔵 LOW: Internal sandbox path structure leaked into model context (bridge_and_annotate): The annotation `[Sandbox copy ava
  • 🔵 LOW: conftest.py session-scoped no-op fixtures may mask test infrastructure regressions (sdk/conftest.py): The added
  • 🔵 LOW: Binary file UTF-8 decode/fallback behavior (e2b_file_tools.py, lines ~375–380): Large binary files (> 32KB) are
  • ...and 4 more
Architect — (15 findings)
  • 🟡 MEDIUM: ✅ Security allowlist pattern: Correctly extends is_allowed_local_path using the existing tuple membership check (`
  • 🟡 MEDIUM: ✅ Best-effort async pattern: bridge_to_sandbox correctly catches all exceptions and only logs, matching the existi
  • 🟡 MEDIUM: ⚠️ Single-file responsibility: e2b_file_tools.py now handles both file tool dispatch and the new bridging concer
  • 🟡 MEDIUM: ❌ Dual bridging paths: bridge_and_annotate is now wired up in two independent read handlers:
  • 🟡 MEDIUM: e2b_file_tools.py_handle_read_file (lines ~148–155 in diff)
  • ...and 10 more
Performance — (13 findings)
  • 🔴 CRITICAL: Base64 inflates content by ~33%
  • 🔴 CRITICAL: Round-trip to E2B API + process launch overhead per write (~100–300 ms typical latency)
  • 🔴 CRITICAL: No batching — each small file is a separate command
  • 🔴 CRITICAL: Per-session bridge cache: Key on (realpath, mtime) so the second Read of the same file within a session skips th
  • 🔴 CRITICAL: Consolidate bridge call sites: Remove the bridge_and_annotate call from one of the two handlers and funnel all tra
  • ...and 8 more
Testing — (2 findings)
  • 🟡 MEDIUM: New code coverage: ~75% estimated
  • 🟡 MEDIUM: Critical paths tested: ⚠️ Mostly covered, with notable gaps
Quality — (16 findings)
  • 🟡 MEDIUM: [e2b_file_tools.py] bridge_to_sandbox and bridge_and_annotate are public (no leading underscore), while all othe
  • 🟡 MEDIUM: [e2b_file_tools.py] The inner function _read_bytes inside bridge_to_sandbox is necessary for asyncio.to_thread
  • 🟡 MEDIUM: [e2b_file_tools.py:~375] The variable data: str | bytes is typed but immediately assigned from a ternary. The expl
  • 🟡 MEDIUM: [tool_adapter_test.py] bridge_calls: list[tuple] — the tuple is untyped. A list[tuple[Any, str, int, int]] or a
  • 🟡 MEDIUM: Duplication of bridging logic across two handlers_handle_read_file in e2b_file_tools.py and `_read_file_handl
  • ...and 11 more
Product — (9 findings)
  • 🟡 MEDIUM: PR claims: Fixes 3 compounding issues preventing large tool output processing in E2B: (1) path= attribute confusio
  • 🟡 MEDIUM: Actual behavior: All three issues are addressed — workspace_path= attribute change in base.py:94, allowlist expa
  • 🟡 MEDIUM: Match: ✅ Complete
  • 🟡 MEDIUM: ✅ N/A — Backend-only change, no UI components
  • 🟡 MEDIUM: ✅ N/A — No keyboard or screen-reader surface area affected
  • ...and 4 more
Discussion — (5 findings)
  • 🟡 MEDIUM: Merge conflicts: Cannot determine (no live API access)
  • 🟡 MEDIUM: Unchecked manual tests: 3 test files — author defers to CI ("CI will validate these")
  • 🟡 MEDIUM: Total comments: Cannot determine (no live API access)
  • 🟡 MEDIUM: Resolved threads: N/A
  • 🟡 MEDIUM: Open threads: N/A

@majdyz
majdyz merged commit 1aef8b7 into dev Apr 2, 2026
25 checks passed
@majdyz
majdyz deleted the fix/copilot-tool-output-e2b-bridging branch April 2, 2026 17:08
@github-project-automation github-project-automation Bot moved this from 🚧 Needs work to ✅ Done in AutoGPT development kanban Apr 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

platform/backend AutoGPT Platform - Back end size/l size/xl

Projects

Status: ✅ Done

Development

Successfully merging this pull request may close these issues.

1 participant