Conversation
Replace the global `max_workspace_storage_mb` config with per-tier limits: - FREE: 250 MB, PRO: 1 GB, BUSINESS: 5 GB, ENTERPRISE: 15 GB Move quota enforcement into WorkspaceManager.write_file() so both REST uploads and CoPilot agent writes are covered (previously only REST had it). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
ntindle
requested review from
Swiftyos and
majdyz
and removed request for
a team
github-project-automation
Bot
moved this to 🆕 Needs initial review
in AutoGPT development kanban
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughEnforces per-user, tier-based workspace storage quotas and centralizes virus scanning in WorkspaceManager.write_file; removes Config-based global storage limit and pre-write scans from routes/tools; adds tier mapping and helper to resolve limits; updates backend tests and frontend storage usage UI. Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant Route as API Route
participant Manager as WorkspaceManager
participant RateLimit as RateLimitService
participant Storage as StorageLayer
Client->>Route: POST /upload_file (content, metadata)
Route->>Manager: write_file(content, metadata...)
Manager->>RateLimit: get_workspace_storage_limit_bytes(user_id)
RateLimit-->>Manager: tier_limit_bytes
Manager->>Storage: get_workspace_total_size(workspace_id)
Storage-->>Manager: current_usage_bytes
alt projected_usage > tier_limit
Manager-->>Route: raises ValueError("Storage limit exceeded...")
Route-->>Client: HTTP 413 Payload Too Large
else proceed to scan and persist
Manager->>Manager: scan_content_safe(content)
alt virus detected
Manager-->>Route: raises VirusDetectedError
Route-->>Client: HTTP 400 Bad Request
else persist
Manager->>Storage: persist file
Storage-->>Manager: success
Manager-->>Route: file metadata
Route-->>Client: HTTP 200 OK
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
/review |
🔍 PR Overlap DetectionThis check compares your PR against all other open PRs targeting the same branch to detect potential merge conflicts early. 🔴 Merge Conflicts DetectedThe following PRs have been tested and will have merge conflicts if merged after this PR. Consider coordinating with the authors.
🟢 Low Risk — File Overlap OnlyThese PRs touch the same files but different sections (click to expand)
Summary: 5 conflict(s), 0 medium risk, 3 low risk (out of 8 PRs with file overlap) Auto-generated on push. Ignores: |
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## dev #12780 +/- ##
==========================================
+ Coverage 69.62% 69.65% +0.03%
==========================================
Files 2135 2136 +1
Lines 157921 158169 +248
Branches 16312 16395 +83
==========================================
+ Hits 109946 110179 +233
- Misses 44727 44736 +9
- Partials 3248 3254 +6
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
autogpt_platform/backend/backend/api/features/workspace/routes.py (1)
263-279:⚠️ Potential issue | 🟠 MajorRoll back the stored blob, not just the DB row.
At Lines 267-272,
soft_delete_workspace_file()only tombstones metadata. The object already written bymanager.write_file()stays in the storage backend, so a rejected concurrent upload still consumes real storage and leaves an orphan. Reusemanager.delete_file(workspace_file.id)here, or explicitly deleteworkspace_file.storage_path, before returning 413.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@autogpt_platform/backend/backend/api/features/workspace/routes.py` around lines 263 - 279, The current rollback only tombstones metadata via soft_delete_workspace_file, leaving the uploaded blob orphaned; after detecting new_total > storage_limit_bytes replace or augment that rollback to also remove the stored object by calling the storage manager's delete (e.g., manager.delete_file(workspace_file.id) or manager.delete_file(workspace_file.storage_path)) inside the same try/except so the blob is removed if present, log any deletion errors (include workspace_file.id and workspace_file.storage_path), and then raise the 413 HTTPException as before.
🤖 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/util/workspace.py`:
- Around line 189-203: The storage check mistakenly treats overwrites as pure
additions; before computing projected usage in the block using
get_workspace_storage_limit_bytes and get_workspace_total_size for
self.workspace_id, resolve the intended target path (the incoming file path) and
fetch the existing file's size (e.g., via the workspace file lookup used
elsewhere in this module) and subtract that existing size_bytes from
current_usage when computing projected usage and warnings so same-size or
smaller overwrites don't get rejected; update the conditional expressions and
warning calculation to use (current_usage - existing_size + len(content))
instead of (current_usage + len(content)).
---
Outside diff comments:
In `@autogpt_platform/backend/backend/api/features/workspace/routes.py`:
- Around line 263-279: The current rollback only tombstones metadata via
soft_delete_workspace_file, leaving the uploaded blob orphaned; after detecting
new_total > storage_limit_bytes replace or augment that rollback to also remove
the stored object by calling the storage manager's delete (e.g.,
manager.delete_file(workspace_file.id) or
manager.delete_file(workspace_file.storage_path)) inside the same try/except so
the blob is removed if present, log any deletion errors (include
workspace_file.id and workspace_file.storage_path), and then raise the 413
HTTPException as before.
🪄 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: fbeb0e6d-92f0-42de-97db-f94b00fe4d2e
📒 Files selected for processing (6)
autogpt_platform/backend/backend/api/features/workspace/routes.pyautogpt_platform/backend/backend/api/features/workspace/routes_test.pyautogpt_platform/backend/backend/copilot/rate_limit.pyautogpt_platform/backend/backend/util/settings.pyautogpt_platform/backend/backend/util/workspace.pyautogpt_platform/backend/backend/util/workspace_test.py
💤 Files with no reviewable changes (1)
- autogpt_platform/backend/backend/util/settings.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). (11)
- GitHub Check: check API types
- GitHub Check: Cursor Bugbot
- GitHub Check: Seer Code Review
- GitHub Check: test (3.12)
- GitHub Check: Analyze (python)
- GitHub Check: type-check (3.11)
- GitHub Check: test (3.11)
- GitHub Check: type-check (3.13)
- GitHub Check: test (3.13)
- GitHub Check: end-to-end tests
- GitHub Check: Check PR Status
🧰 Additional context used
📓 Path-based instructions (5)
autogpt_platform/backend/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
autogpt_platform/backend/**/*.py: Use Python 3.11 (required; managed by Poetry via pyproject.toml) for backend development
Always run 'poetry run format' (Black + isort) before linting in backend development
Always run 'poetry run lint' (ruff) after formatting in backend development
autogpt_platform/backend/**/*.py: Usepoetry run ...command for executing Python package dependencies
Use top-level imports only — avoid local/inner imports except for lazy imports of heavy optional dependencies likeopenpyxl
Use absolute imports withfrom backend.module import ...for cross-package imports; single-dot relative imports are acceptable for sibling modules within the same package; avoid double-dot relative imports
Do not use duck typing — avoidhasattr/getattr/isinstancefor type dispatch; use typed interfaces/unions/protocols instead
Use Pydantic models over dataclass/namedtuple/dict for structured data
Do not use linter suppressors — no# type: ignore,# noqa,# pyright: ignore; fix the type/code instead
Prefer list comprehensions over manual loop-and-append patterns
Use early return with guard clauses first to avoid deep nesting
Use%sfor deferred interpolation indebuglog statements for efficiency; use f-strings elsewhere for readability (e.g.,logger.debug("Processing %s items", count)vslogger.info(f"Processing {count} items"))
Sanitize error paths by usingos.path.basename()in error messages to avoid leaking directory structure
Be aware of TOCTOU (Time-Of-Check-Time-Of-Use) issues — avoid check-then-act patterns for file access and credit charging
Usetransaction=Truefor Redis pipelines to ensure atomicity on multi-step operations
Usemax(0, value)guards for computed values that should never be negative
Keep files under ~300 lines; if a file grows beyond this, split by responsibility (extract helpers, models, or a sub-module into a new file)
Keep functions under ~40 lines; extract named helpers when a function grows longer
...
Files:
autogpt_platform/backend/backend/api/features/workspace/routes_test.pyautogpt_platform/backend/backend/util/workspace_test.pyautogpt_platform/backend/backend/copilot/rate_limit.pyautogpt_platform/backend/backend/util/workspace.pyautogpt_platform/backend/backend/api/features/workspace/routes.py
autogpt_platform/backend/backend/api/features/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Update routes in '/backend/backend/api/features/' and add/update Pydantic models in the same directory for API development
Files:
autogpt_platform/backend/backend/api/features/workspace/routes_test.pyautogpt_platform/backend/backend/api/features/workspace/routes.py
autogpt_platform/{backend,autogpt_libs}/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Format Python code with
poetry run format
Files:
autogpt_platform/backend/backend/api/features/workspace/routes_test.pyautogpt_platform/backend/backend/util/workspace_test.pyautogpt_platform/backend/backend/copilot/rate_limit.pyautogpt_platform/backend/backend/util/workspace.pyautogpt_platform/backend/backend/api/features/workspace/routes.py
autogpt_platform/backend/**/api/**/*.py
📄 CodeRabbit inference engine (autogpt_platform/backend/AGENTS.md)
autogpt_platform/backend/**/api/**/*.py: UseSecurity()instead ofDepends()for authentication dependencies to get proper OpenAPI security specification
Follow SSE (Server-Sent Events) protocol: usedata:lines for frontend-parsed events (must match Zod schema) and: commentlines for heartbeats/status
Files:
autogpt_platform/backend/backend/api/features/workspace/routes_test.pyautogpt_platform/backend/backend/api/features/workspace/routes.py
autogpt_platform/backend/**/*_test.py
📄 CodeRabbit inference engine (autogpt_platform/backend/AGENTS.md)
autogpt_platform/backend/**/*_test.py: Use pytest with snapshot testing for API responses
Colocate test files with source files using*_test.pynaming convention
Mock at boundaries — mock where the symbol is used, not where it's defined; after refactoring, update mock targets to match new module paths
UseAsyncMockfromunittest.mockfor async functions in tests
When writing tests, use Test-Driven Development (TDD): write failing tests marked with@pytest.mark.xfailbefore implementation, then remove the marker once the implementation is complete
When creating snapshots in tests, usepoetry run pytest path/to/test.py --snapshot-update; always review snapshot changes withgit diffbefore committing
Files:
autogpt_platform/backend/backend/api/features/workspace/routes_test.pyautogpt_platform/backend/backend/util/workspace_test.py
🧠 Learnings (18)
📓 Common learnings
Learnt from: Pwuts
Repo: Significant-Gravitas/AutoGPT PR: 12284
File: autogpt_platform/frontend/src/app/api/openapi.json:11897-11900
Timestamp: 2026-03-04T23:58:18.476Z
Learning: Repo: Significant-Gravitas/AutoGPT — PR `#12284`
Backend/frontend OpenAPI codegen convention: In backend/api/features/store/model.py, the StoreSubmission and StoreSubmissionAdminView models define submitted_at: datetime | None, changes_summary: str | None, and instructions: str | None with no default. This is intentional to produce “required but nullable” fields in OpenAPI (properties appear in required[] and use anyOf [type, null]). This matches Prisma’s submittedAt DateTime? and changesSummary String?. Do not flag this as a required/nullable mismatch.
📚 Learning: 2026-04-08T17:28:23.439Z
Learnt from: CR
Repo: Significant-Gravitas/AutoGPT PR: 0
File: autogpt_platform/backend/AGENTS.md:0-0
Timestamp: 2026-04-08T17:28:23.439Z
Learning: Applies to autogpt_platform/backend/**/*_test.py : Mock at boundaries — mock where the symbol is **used**, not where it's **defined**; after refactoring, update mock targets to match new module paths
Applied to files:
autogpt_platform/backend/backend/api/features/workspace/routes_test.pyautogpt_platform/backend/backend/util/workspace_test.py
📚 Learning: 2026-02-04T16:49:42.490Z
Learnt from: CR
Repo: Significant-Gravitas/AutoGPT PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-02-04T16:49:42.490Z
Learning: Applies to autogpt_platform/backend/**/test/**/*.py : Use snapshot testing with '--snapshot-update' flag in backend tests when output changes; always review with 'git diff'
Applied to files:
autogpt_platform/backend/backend/api/features/workspace/routes_test.pyautogpt_platform/backend/backend/util/workspace_test.py
📚 Learning: 2026-04-08T17:28:23.439Z
Learnt from: CR
Repo: Significant-Gravitas/AutoGPT PR: 0
File: autogpt_platform/backend/AGENTS.md:0-0
Timestamp: 2026-04-08T17:28:23.439Z
Learning: Applies to autogpt_platform/backend/**/*_test.py : When creating snapshots in tests, use `poetry run pytest path/to/test.py --snapshot-update`; always review snapshot changes with `git diff` before committing
Applied to files:
autogpt_platform/backend/backend/api/features/workspace/routes_test.pyautogpt_platform/backend/backend/util/workspace_test.py
📚 Learning: 2026-03-19T15:10:53.815Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12483
File: autogpt_platform/backend/backend/copilot/tools/test_dry_run.py:298-303
Timestamp: 2026-03-19T15:10:53.815Z
Learning: In Python unittest.mock, the correct patch target depends on whether an import is eager (module-level) or lazy (inside a function/branch):
- **Module-level import** (`from foo.bar import baz` at top of file): patch where the name is used, e.g. `patch("mymodule.baz")`.
- **Lazy import** (`from foo.bar import baz` inside a function/branch, executed at call time): patch the source module, e.g. `patch("foo.bar.baz")`, because the fresh `from ... import` at call time will look up the (now-patched) name in the source module's dict.
This pattern appears in `autogpt_platform/backend/backend/copilot/tools/helpers.py` where `simulate_block` is lazily imported inside the `if dry_run:` block, making `patch("backend.executor.simulator.simulate_block")` the correct target in tests.
Applied to files:
autogpt_platform/backend/backend/api/features/workspace/routes_test.pyautogpt_platform/backend/backend/util/workspace_test.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/api/features/workspace/routes_test.pyautogpt_platform/backend/backend/util/workspace_test.pyautogpt_platform/backend/backend/copilot/rate_limit.pyautogpt_platform/backend/backend/util/workspace.pyautogpt_platform/backend/backend/api/features/workspace/routes.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/api/features/workspace/routes_test.pyautogpt_platform/backend/backend/util/workspace_test.pyautogpt_platform/backend/backend/copilot/rate_limit.pyautogpt_platform/backend/backend/util/workspace.pyautogpt_platform/backend/backend/api/features/workspace/routes.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/api/features/workspace/routes_test.pyautogpt_platform/backend/backend/util/workspace_test.pyautogpt_platform/backend/backend/copilot/rate_limit.pyautogpt_platform/backend/backend/util/workspace.pyautogpt_platform/backend/backend/api/features/workspace/routes.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/api/features/workspace/routes_test.pyautogpt_platform/backend/backend/util/workspace_test.pyautogpt_platform/backend/backend/copilot/rate_limit.pyautogpt_platform/backend/backend/util/workspace.pyautogpt_platform/backend/backend/api/features/workspace/routes.py
📚 Learning: 2026-04-08T17:26:50.234Z
Learnt from: CR
Repo: Significant-Gravitas/AutoGPT PR: 0
File: classic/direct_benchmark/CLAUDE.md:0-0
Timestamp: 2026-04-08T17:26:50.234Z
Learning: Implement workspace-based isolation instead of port-based isolation for parallel test execution
Applied to files:
autogpt_platform/backend/backend/util/workspace_test.py
📚 Learning: 2026-04-01T04:17:43.495Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12632
File: autogpt_platform/backend/backend/copilot/tools/workspace_files.py:0-0
Timestamp: 2026-04-01T04:17:43.495Z
Learning: In autogpt_platform/backend/backend/copilot/sdk/tool_adapter.py (PR `#12632`, commit 12ae03c), the per-tool `BaseTool.read_only` property approach was removed. Instead, `readOnlyHint=True` (via `ToolAnnotations`) is applied unconditionally to ALL tools — including side-effect tools like `bash_exec` and `write_workspace_file` — to enable fully parallel dispatch by the Anthropic SDK/CLI. Do not flag tools with mutating operations (e.g. save_to_path, write operations) for having `readOnlyHint=True`; this is intentional and E2E validated (3x bash_exec(sleep 3) completed in 3.3s vs 9s sequential).
Applied to files:
autogpt_platform/backend/backend/util/workspace_test.py
📚 Learning: 2026-03-13T15:49:44.961Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12385
File: autogpt_platform/backend/backend/copilot/rate_limit.py:0-0
Timestamp: 2026-03-13T15:49:44.961Z
Learning: In `autogpt_platform/backend/backend/copilot/rate_limit.py`, the original per-session token window (with a TTL-based reset) was replaced with fixed daily and weekly windows. `resets_at` is now derived from `_daily_reset_time()` (midnight UTC) and `_weekly_reset_time()` (next Monday 00:00 UTC) — deterministic fixed-boundary calculations that require no Redis TTL introspection.
Applied to files:
autogpt_platform/backend/backend/copilot/rate_limit.py
📚 Learning: 2026-04-03T13:50:29.037Z
Learnt from: Pwuts
Repo: Significant-Gravitas/AutoGPT PR: 12206
File: autogpt_platform/backend/backend/api/external/v2/rate_limit.py:24-56
Timestamp: 2026-04-03T13:50:29.037Z
Learning: In `autogpt_platform/backend/backend/api/external/v2/rate_limit.py`, the `RateLimiter` class uses in-process (per-worker) memory for sliding-window rate limiting. This is intentionally documented as a known limitation via WARNING comments in the module and class docstrings. A full Redis-backed migration (using ZADD/ZREMRANGEBYSCORE/ZCARD with TTL/Lua for atomic multi-replica enforcement) is deferred to a later PR. Do not re-flag the in-memory implementation as a blocking bug — the limitation is documented and accepted for the initial v2 external API release.
Applied to files:
autogpt_platform/backend/backend/copilot/rate_limit.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/rate_limit.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/rate_limit.py
📚 Learning: 2026-03-23T06:51:32.535Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12440
File: autogpt_platform/backend/backend/api/features/workflow_import.py:0-0
Timestamp: 2026-03-23T06:51:32.535Z
Learning: In Significant-Gravitas/AutoGPT PR `#12440`, the dedicated REST endpoint `/api/import/workflow` (and its earlier form `/api/import/competitor-workflow`) for external workflow import was removed across commits 30f801a5e, 374c8cfdb, and 732960e2d. The final architecture is a frontend-only flow: file upload is handled client-side, and URL fetching (for n8n templates) uses a Next.js server action (`fetchWorkflowFromUrl`) — there is no dedicated backend HTTP endpoint and no CoPilot tool invocation for workflow import. Do not expect or require a standalone HTTP endpoint, route-level integration tests, or a CoPilot `import_workflow` tool for workflow import in this repository.
Applied to files:
autogpt_platform/backend/backend/api/features/workspace/routes.py
📚 Learning: 2026-03-24T21:27:22.326Z
Learnt from: ntindle
Repo: Significant-Gravitas/AutoGPT PR: 12536
File: autogpt_platform/frontend/src/app/api/openapi.json:5732-5752
Timestamp: 2026-03-24T21:27:22.326Z
Learning: Repo: Significant-Gravitas/AutoGPT — Preference: Do not add explicit 403/404 entries to FastAPI route decorators for admin endpoints just to influence OpenAPI. Keep openapi.json autogenerated and use route docstrings to document admin-only (403) and not-found (404) behavior; rely on tests for enforcement. File context: autogpt_platform/backend/backend/api/features/admin/store_admin_routes.py. PR `#12536`.
Applied to files:
autogpt_platform/backend/backend/api/features/workspace/routes.py
📚 Learning: 2026-02-04T16:49:42.490Z
Learnt from: CR
Repo: Significant-Gravitas/AutoGPT PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-02-04T16:49:42.490Z
Learning: Applies to autogpt_platform/backend/backend/api/features/**/*.py : Update routes in '/backend/backend/api/features/' and add/update Pydantic models in the same directory for API development
Applied to files:
autogpt_platform/backend/backend/api/features/workspace/routes.py
🔇 Additional comments (1)
autogpt_platform/backend/backend/util/workspace.py (1)
190-192:⚠️ Potential issue | 🟠 MajorQuota enforcement is still non-atomic for direct
WorkspaceManager.write_file()callers.At Lines 190-192,
write_file()snapshots total usage and only writes later. The compensating post-write rollback lives inautogpt_platform/backend/backend/api/features/workspace/routes.py, so any caller that usesWorkspaceManager.write_file()directly can still race past the tier cap under concurrency. Move the authoritative quota reservation/rollback intoWorkspaceManager.write_file()or serialize writes per workspace. As per coding guidelines, "Be aware of TOCTOU (Time-Of-Check-Time-Of-Use) issues — avoid check-then-act patterns for file access and credit charging."⛔ Skipped due to learnings
Learnt from: CR Repo: Significant-Gravitas/AutoGPT PR: 0 File: autogpt_platform/backend/AGENTS.md:0-0 Timestamp: 2026-04-08T17:28:23.439Z Learning: Applies to autogpt_platform/backend/**/*.py : Be aware of TOCTOU (Time-Of-Check-Time-Of-Use) issues — avoid check-then-act patterns for file access and credit chargingLearnt from: majdyz Repo: Significant-Gravitas/AutoGPT PR: 12750 File: autogpt_platform/backend/backend/copilot/sdk/file_tools.py:0-0 Timestamp: 2026-04-11T23:51:20.769Z Learning: In `autogpt_platform/backend/backend/copilot/sdk/file_tools.py`, the TOCTOU gap between `is_allowed_local_path(resolved, sdk_cwd)` and the subsequent `open()` in `_handle_write_non_e2b` (and analogous read/edit handlers) is an accepted trade-off. In non-E2B mode the SDK working directory is ephemeral and user-scoped (created per session under `/tmp`), making the practical attack surface minimal. The E2B sandbox already uses `readlink -f` for defense-in-depth. Additional TOCTOU hardening (e.g., `O_NOFOLLOW`) should be reconsidered only if/when persistent workspaces are introduced. Do not flag this as a blocking issue for non-E2B ephemeral workspace paths.Learnt from: majdyz Repo: Significant-Gravitas/AutoGPT PR: 12691 File: .claude/skills/orchestrate/scripts/poll-cycle.sh:29-30 Timestamp: 2026-04-07T09:54:36.517Z Learning: In Significant-Gravitas/AutoGPT, `.claude/skills/orchestrate/scripts/poll-cycle.sh` uses an atomic `.tmp` + `mv` write pattern for `~/.claude/orchestrator-state.json`. The lack of `flock`-based serialization across concurrent writers (poll-cycle.sh, add, stop) is an accepted trade-off because the CronCreate polling interval is ~2 minutes, making true races low-probability. Using `flock` on the state file plus per-writer `mktemp` temp paths is noted as a future improvement, not a current requirement.Learnt from: majdyz Repo: Significant-Gravitas/AutoGPT PR: 12385 File: autogpt_platform/backend/backend/copilot/rate_limit.py:141-170 Timestamp: 2026-03-12T14:42:40.552Z Learning: In Significant-Gravitas/AutoGPT, `check_rate_limit` in `autogpt_platform/backend/backend/copilot/rate_limit.py` is intentionally a "pre-turn soft check" (not a hard atomic reservation). Because LLM token counts are unknown before generation completes, a strict check-and-reserve is impractical. The TOCTOU race (two concurrent turns both passing the pre-check and both committing via `record_token_usage`) is an accepted trade-off. If stricter enforcement is ever needed, the approach is a Lua script doing GET+INCRBY atomically in Redis.Learnt from: Pwuts Repo: Significant-Gravitas/AutoGPT PR: 12206 File: autogpt_platform/backend/backend/api/external/v2/rate_limit.py:24-56 Timestamp: 2026-04-03T13:50:29.037Z Learning: In `autogpt_platform/backend/backend/api/external/v2/rate_limit.py`, the `RateLimiter` class uses in-process (per-worker) memory for sliding-window rate limiting. This is intentionally documented as a known limitation via WARNING comments in the module and class docstrings. A full Redis-backed migration (using ZADD/ZREMRANGEBYSCORE/ZCARD with TTL/Lua for atomic multi-replica enforcement) is deferred to a later PR. Do not re-flag the in-memory implementation as a blocking bug — the limitation is documented and accepted for the initial v2 external API release.Learnt from: majdyz Repo: Significant-Gravitas/AutoGPT PR: 12632 File: autogpt_platform/backend/backend/copilot/tools/workspace_files.py:0-0 Timestamp: 2026-04-01T04:17:43.495Z Learning: In autogpt_platform/backend/backend/copilot/sdk/tool_adapter.py (PR `#12632`, commit 12ae03c), the per-tool `BaseTool.read_only` property approach was removed. Instead, `readOnlyHint=True` (via `ToolAnnotations`) is applied unconditionally to ALL tools — including side-effect tools like `bash_exec` and `write_workspace_file` — to enable fully parallel dispatch by the Anthropic SDK/CLI. Do not flag tools with mutating operations (e.g. save_to_path, write operations) for having `readOnlyHint=True`; this is intentional and E2E validated (3x bash_exec(sleep 3) completed in 3.3s vs 9s sequential).
![autogpt-pr-reviewer[bot]](https://avatars.githubusercontent.com/u/130738209?s=60&v=4){kind=small}
There was a problem hiding this comment.
📋 Automated Review — PR #12780
PR #12780 — feat(backend): tier-based workspace file storage limits
Author: ntindle | Files: 6
🎯 Verdict: REQUEST_CHANGES
PR Description Quality
✅ Has Why + What + How — PR describes the motivation (CoPilot bypass of storage limits), the approach (move enforcement into WorkspaceManager.write_file()), and the tier mapping.
What This PR Does
Previously, workspace file storage had a flat 500 MB limit enforced only at the REST route level, meaning CoPilot file writes bypassed quota checks entirely. This PR replaces the flat limit with tier-based storage caps (FREE: 250 MB, PRO: 1 GB, BUSINESS: 5 GB, ENTERPRISE: 15 GB) and moves quota enforcement into WorkspaceManager.write_file() so both REST and CoPilot paths are covered. It also adds an 80% usage warning log and a post-write TOCTOU safety net on the REST path.
Specialist Findings
🛡️ Security ✅ — Core improvement is sound: quota enforcement centralized, JWT-authenticated user_id used for tier lookup, fail-safe fallback to FREE tier on DB errors. No auth bypass or injection vectors.
- 🟡 TOCTOU race partially closed — REST route has post-write safety net but CoPilot path does not (
workspace.py:190-197). Practical impact is low (requires precise race timing, single-user scope).
🏗️ Architecture TIER_MULTIPLIERS pattern.
- 🟠 String-based error dispatch (
routes.py:257) — HTTP status codes determined byValueErrormessage prefix matching. A refactored error message silently breaks status mapping. Custom exception types needed. (Flagged by: architect, quality — 2) - 🟠 Dependency inversion (
workspace.py:15) —utillayer imports fromcopilot.rate_limit, inverting expected layering. Storage limits aren't CoPilot-specific. (Flagged by: architect — 1) - 🟡
VirusDetectedErrordefined instore.exceptionsbut now raised fromutil.workspace— misplaced across layers (routes.py:14).
⚡ Performance
- 🟠
get_workspace_total_size()fetches allUserWorkspaceFilerows into Python tosum()instead of using a DB aggregate — O(N) per upload, now called 2× per REST upload (data/workspace.py:352-355). Enterprise users with thousands of files will feel this. (Flagged by: performance — 1) - 🟡 Double
get_workspace_storage_limit_bytes()call per REST upload (workspace.py:190+routes.py:263) — two Redis round-trips for the same value. (Flagged by: performance, architect, quality — 3)
🧪 Testing
- 🔴
get_workspace_storage_limit_byteshas no unit test — the tier→bytes mapping is the centerpiece of this PR and is never verified (rate_limit.py:458). (Flagged by: testing — 1) - 🔴
WorkspaceManager.write_file()quota rejection never exercised — all 3 workspace tests mock usage to 0 and limit to 250 MB, so theStorage limit exceededpath atworkspace.py:192is untested. (Flagged by: testing — 1) - 🟠 80% warning path untested (
workspace.py:198),get_storage_usageendpoint untested with tier-based limit (routes.py:296). codecov patch coverage failing at 77.8%.
📖 Quality ✅ — Clean naming, good inline comments, consistent with codebase style.
- 🟠 Test mock message (
routes_test.py:269) uses"500 MB used of 250 MB"but real code produces"524,288,000 bytes"format — reduces documentary value. (Flagged by: quality — 1) - 🔵 Repeated sub-expression
current_usage + len(content)atworkspace.py:191,197— extract toprojected_usagevariable. - 🔵 Redundant
if storage_limit andguard atworkspace.py:197— always truthy since minimum tier is 250 MB.
📦 Product
- 🟠 FREE tier drops from 500 MB → 250 MB with no migration/grandfathering plan (
rate_limit.py:63). Users currently at 250–500 MB are immediately locked out. (Flagged by: product — 1) - 🟠 Error messages expose raw byte counts (
"262,144,000 bytes") instead of human-readable sizes. No upgrade guidance. (workspace.py:192-195). (Flagged by: product, security — 2) - 🟡 80% warning is server-log-only — users have no visibility into approaching their limit (
workspace.py:197).
📬 Discussion
- 🔴
VirusScanError(extendsValueError) caught by genericexcept ValueError→ mapped to HTTP 409 instead of 500 on ClamAV outage (routes.py:251-258). Flagged by sentry[bot] and cursor[bot], no author response. This is a real bug. - 🔴 Duplicate virus scan in CoPilot tool —
WriteWorkspaceFileTool._execute()still callsscan_content_safebeforemanager.write_file(), causing double scanning. Route was cleaned up but CoPilot tool was not. Flagged by cursor[bot], no response. - 🟠 Overwrite quota miscalculation —
current_usagealready includes the file being replaced, so overwrites of same-size files are incorrectly rejected near the cap (workspace.py:189). Flagged by coderabbitai[bot], no response.
🔎 QA ✅ — Live API testing confirms tier-based limits work correctly. Upload, overwrite, conflict detection, and auth enforcement all verified against running stack. PRO tier correctly returns 1 GB limit.
🔴 Blockers
-
VirusScanErrorsilently mapped to 409 (routes.py:251-258) —VirusScanErrorextendsValueError, so ClamAV infrastructure failures (scan service down) get caught by the genericexcept ValueErrorhandler and return HTTP 409 Conflict instead of 500. Users see "conflict" when the real problem is a backend outage. Addexcept VirusScanErrorbeforeexcept ValueError. (Flagged by: discussion/sentry[bot], discussion/cursor[bot] — 2) -
Duplicate virus scan on CoPilot path (
workspace_files.py:~840) —WriteWorkspaceFileTool._execute()still callsscan_content_safebeforemanager.write_file(), butwrite_file()now owns scanning internally. Every CoPilot file write is virus-scanned twice. Remove the pre-call scan in the CoPilot tool. (Flagged by: discussion/cursor[bot] — 1) -
Zero test coverage for core quota logic (
rate_limit.py:458,workspace.py:192) —get_workspace_storage_limit_byteshas no unit test. The quota rejection path inwrite_file()is never exercised (all tests mock usage to 0). The codecov coverage gate is failing. Add: (a) parametrized test for tier→bytes mapping, (b) test triggeringStorage limit exceededValueError, (c) test for 80% warning log. (Flagged by: testing — 1)
🟠 Should Fix
-
Overwrite quota miscalculation (
workspace.py:189-191) —current_usageincludes the file being overwritten, so replacing a 10 MB file with a 10 MB file near the cap is incorrectly rejected. Subtract the existing file's size whenoverwrite=True. (Flagged by: discussion/coderabbitai — 1) -
String-based error dispatch (
routes.py:257) — HTTP status codes depend onValueErrormessage prefix matching ("File too large","Storage limit exceeded"). Define typed exceptions (FileTooLargeError,StorageQuotaExceededError) for type-safe dispatch. (Flagged by: architect, quality — 2) -
O(N) total-size query (
data/workspace.py:352-355) —get_workspace_total_size()loads all file rows into Python to sum. UseSELECT SUM(size_bytes)aggregate. Now called 2× per REST upload, making this O(2N) per upload. (Flagged by: performance — 1) -
FREE tier 50% reduction without migration (
rate_limit.py:63) — Users between 250–500 MB are immediately locked out. Needs a migration plan, grace period, or grandfathering. (Flagged by: product — 1) -
Error messages not user-friendly (
workspace.py:192-195) — Raw byte counts like"262,144,000 bytes"exposed to users via HTTP 413. Format as human-readable sizes with upgrade guidance. (Flagged by: product, security — 2)
🟡 Nice to Have
- Extract storage limit to shared module (
workspace.py:15) —util→copilotdependency inversion. MoveTIER_WORKSPACE_STORAGE_MBtobackend.util.subscription. (architect) - Return resolved
storage_limitfromwrite_file()(routes.py:263) — Avoids redundant tier lookup in TOCTOU check. (performance, architect, quality) - Add tier name to storage usage response (
routes.py:304) — Enables frontend upgrade prompts. (product) - Surface 80% warning to users (
workspace.py:197) — Currently server-log-only; add response header or field. (product)
🔵 Nits
- Redundant truthiness guard (
workspace.py:197) —if storage_limit and ...is always true; minimum tier = 250 MB. - Repeated sub-expression (
workspace.py:191,197) — Extractprojected_usage = current_usage + len(content). - Test mock message format mismatch (
routes_test.py:269) — Mock says "500 MB" but real code produces "524,288,000 bytes".
QA Screenshots
| Screenshot | Description |
|---|---|
 |
Full stack health check — copilot page loads without errors after login ✅ |
Human Review Needed
YES — Security-sensitive change (storage quota enforcement, billing-tier logic), breaking change for FREE users (50% limit reduction), unaddressed bot findings including a real bug (VirusScanError → 409), and zero human approvals so far.
Risk Assessment
Merge risk: MEDIUM | Rollback: EASY (no DB migrations, feature is additive, old flat limit can be restored by reverting)
CI Status
❌ 2/35 checks failed — codecov/patch/Platform Backend (77.8% patch coverage below threshold), Check PR Status (meta-check gated on codecov). Backend lint passes. Frontend checks fail but are unrelated to this backend-only PR.
github-project-automation
Bot
moved this from 🆕 Needs initial review
to 🚧 Needs work
in AutoGPT development kanban
…erage - Fix VirusScanError (ClamAV outage) returning 409 instead of 500 - Fix duplicate virus scan in CoPilot WriteWorkspaceFileTool - Fix overwrite quota miscalculation (subtract old file size from usage) - Add parametrized tests for get_workspace_storage_limit_bytes (all tiers) - Add test for quota rejection path in WorkspaceManager.write_file() - Add test for 80% warning log - Add test for VirusScanError → 500 mapping - Add test for get_storage_usage endpoint with tier-based limit - Extract projected_usage variable, remove redundant truthiness guard Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
autogpt_platform/backend/backend/util/workspace_test.py (1)
233-236: Moveimport loggingto module scope.This is a regular stdlib import, so keeping it local just adds noise and violates the backend top-level-import rule.
As per coding guidelines: "Use top-level imports only — avoid local/inner imports except for lazy imports of heavy optional dependencies like
openpyxl"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@autogpt_platform/backend/backend/util/workspace_test.py` around lines 233 - 236, The test currently does a local import of the stdlib logging module inside the test body; move "import logging" to the module-level imports at the top of workspace_test.py and remove the local import so the test uses the top-level logging symbol (the caplog block around manager.write_file remains unchanged); this addresses the backend top-level-import rule and keeps the test using logger="backend.util.workspace" intact.
🤖 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/util/workspace.py`:
- Around line 223-225: The duplicate-write guard is being bypassed because
scan_content_safe(...) is called before checking for existing files when
overwrite=False; reorder the logic in the WorkspaceManager file-save flow so
that when overwrite is False you first perform the cheap duplicate-path
existence check (the "File already exists" guard) and return the deterministic
error before calling scan_content_safe; update both the earlier occurrence and
the later similar block (the other instance around lines 233-236) to perform
existence checks prior to invoking scan_content_safe, referencing the overwrite
flag, the duplicate-path guard, and scan_content_safe to locate the spots to
change.
- Around line 198-221: The quota check in WorkspaceManager.write_file is
TOCTOU-prone because it samples get_workspace_total_size() before the write; fix
by making the check-and-reserve atomic in the DB or adding a post-write
rollback: implement a transactional reserve step in the same DB transaction that
will insert/update the file row only if (current_total + added_size -
replaced_size) <= storage_limit (use workspace_db() methods or a new DB-level
conditional update / "reserve_bytes" operation), or if DB-level atomicity isn't
available add the same post-write safety net used by the REST route to detect
over-quota and roll back the insert/update and file write; update references in
code to get_workspace_storage_limit_bytes, get_workspace_total_size,
workspace_db().get_workspace_file_by_path, and WorkspaceManager.write_file to
use the atomic reservation or rollback path.
---
Nitpick comments:
In `@autogpt_platform/backend/backend/util/workspace_test.py`:
- Around line 233-236: The test currently does a local import of the stdlib
logging module inside the test body; move "import logging" to the module-level
imports at the top of workspace_test.py and remove the local import so the test
uses the top-level logging symbol (the caplog block around manager.write_file
remains unchanged); this addresses the backend top-level-import rule and keeps
the test using logger="backend.util.workspace" intact.
🪄 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: 780c503d-109b-4538-b14b-792a7f62eb08
📒 Files selected for processing (6)
autogpt_platform/backend/backend/api/features/workspace/routes.pyautogpt_platform/backend/backend/api/features/workspace/routes_test.pyautogpt_platform/backend/backend/copilot/rate_limit_test.pyautogpt_platform/backend/backend/copilot/tools/workspace_files.pyautogpt_platform/backend/backend/util/workspace.pyautogpt_platform/backend/backend/util/workspace_test.py
💤 Files with no reviewable changes (1)
- autogpt_platform/backend/backend/copilot/tools/workspace_files.py
🚧 Files skipped from review as they are similar to previous changes (2)
- autogpt_platform/backend/backend/api/features/workspace/routes.py
- autogpt_platform/backend/backend/api/features/workspace/routes_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). (12)
- GitHub Check: check API types
- GitHub Check: Seer Code Review
- GitHub Check: Cursor Bugbot
- GitHub Check: end-to-end tests
- GitHub Check: test (3.12)
- GitHub Check: test (3.11)
- GitHub Check: type-check (3.12)
- GitHub Check: type-check (3.11)
- GitHub Check: type-check (3.13)
- GitHub Check: test (3.13)
- GitHub Check: Check PR Status
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (3)
autogpt_platform/backend/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
autogpt_platform/backend/**/*.py: Use Python 3.11 (required; managed by Poetry via pyproject.toml) for backend development
Always run 'poetry run format' (Black + isort) before linting in backend development
Always run 'poetry run lint' (ruff) after formatting in backend development
autogpt_platform/backend/**/*.py: Usepoetry run ...command for executing Python package dependencies
Use top-level imports only — avoid local/inner imports except for lazy imports of heavy optional dependencies likeopenpyxl
Use absolute imports withfrom backend.module import ...for cross-package imports; single-dot relative imports are acceptable for sibling modules within the same package; avoid double-dot relative imports
Do not use duck typing — avoidhasattr/getattr/isinstancefor type dispatch; use typed interfaces/unions/protocols instead
Use Pydantic models over dataclass/namedtuple/dict for structured data
Do not use linter suppressors — no# type: ignore,# noqa,# pyright: ignore; fix the type/code instead
Prefer list comprehensions over manual loop-and-append patterns
Use early return with guard clauses first to avoid deep nesting
Use%sfor deferred interpolation indebuglog statements for efficiency; use f-strings elsewhere for readability (e.g.,logger.debug("Processing %s items", count)vslogger.info(f"Processing {count} items"))
Sanitize error paths by usingos.path.basename()in error messages to avoid leaking directory structure
Be aware of TOCTOU (Time-Of-Check-Time-Of-Use) issues — avoid check-then-act patterns for file access and credit charging
Usetransaction=Truefor Redis pipelines to ensure atomicity on multi-step operations
Usemax(0, value)guards for computed values that should never be negative
Keep files under ~300 lines; if a file grows beyond this, split by responsibility (extract helpers, models, or a sub-module into a new file)
Keep functions under ~40 lines; extract named helpers when a function grows longer
...
Files:
autogpt_platform/backend/backend/util/workspace.pyautogpt_platform/backend/backend/copilot/rate_limit_test.pyautogpt_platform/backend/backend/util/workspace_test.py
autogpt_platform/{backend,autogpt_libs}/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Format Python code with
poetry run format
Files:
autogpt_platform/backend/backend/util/workspace.pyautogpt_platform/backend/backend/copilot/rate_limit_test.pyautogpt_platform/backend/backend/util/workspace_test.py
autogpt_platform/backend/**/*_test.py
📄 CodeRabbit inference engine (autogpt_platform/backend/AGENTS.md)
autogpt_platform/backend/**/*_test.py: Use pytest with snapshot testing for API responses
Colocate test files with source files using*_test.pynaming convention
Mock at boundaries — mock where the symbol is used, not where it's defined; after refactoring, update mock targets to match new module paths
UseAsyncMockfromunittest.mockfor async functions in tests
When writing tests, use Test-Driven Development (TDD): write failing tests marked with@pytest.mark.xfailbefore implementation, then remove the marker once the implementation is complete
When creating snapshots in tests, usepoetry run pytest path/to/test.py --snapshot-update; always review snapshot changes withgit diffbefore committing
Files:
autogpt_platform/backend/backend/copilot/rate_limit_test.pyautogpt_platform/backend/backend/util/workspace_test.py
🧠 Learnings (17)
📓 Common learnings
Learnt from: Pwuts
Repo: Significant-Gravitas/AutoGPT PR: 12284
File: autogpt_platform/frontend/src/app/api/openapi.json:11897-11900
Timestamp: 2026-03-04T23:58:18.476Z
Learning: Repo: Significant-Gravitas/AutoGPT — PR `#12284`
Backend/frontend OpenAPI codegen convention: In backend/api/features/store/model.py, the StoreSubmission and StoreSubmissionAdminView models define submitted_at: datetime | None, changes_summary: str | None, and instructions: str | None with no default. This is intentional to produce “required but nullable” fields in OpenAPI (properties appear in required[] and use anyOf [type, null]). This matches Prisma’s submittedAt DateTime? and changesSummary String?. Do not flag this as a required/nullable mismatch.
📚 Learning: 2026-04-11T23:51:20.769Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12750
File: autogpt_platform/backend/backend/copilot/sdk/file_tools.py:0-0
Timestamp: 2026-04-11T23:51:20.769Z
Learning: In `autogpt_platform/backend/backend/copilot/sdk/file_tools.py`, the TOCTOU gap between `is_allowed_local_path(resolved, sdk_cwd)` and the subsequent `open()` in `_handle_write_non_e2b` (and analogous read/edit handlers) is an accepted trade-off. In non-E2B mode the SDK working directory is ephemeral and user-scoped (created per session under `/tmp`), making the practical attack surface minimal. The E2B sandbox already uses `readlink -f` for defense-in-depth. Additional TOCTOU hardening (e.g., `O_NOFOLLOW`) should be reconsidered only if/when persistent workspaces are introduced. Do not flag this as a blocking issue for non-E2B ephemeral workspace paths.
Applied to files:
autogpt_platform/backend/backend/util/workspace.py
📚 Learning: 2026-04-01T04:17:43.495Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12632
File: autogpt_platform/backend/backend/copilot/tools/workspace_files.py:0-0
Timestamp: 2026-04-01T04:17:43.495Z
Learning: In autogpt_platform/backend/backend/copilot/sdk/tool_adapter.py (PR `#12632`, commit 12ae03c), the per-tool `BaseTool.read_only` property approach was removed. Instead, `readOnlyHint=True` (via `ToolAnnotations`) is applied unconditionally to ALL tools — including side-effect tools like `bash_exec` and `write_workspace_file` — to enable fully parallel dispatch by the Anthropic SDK/CLI. Do not flag tools with mutating operations (e.g. save_to_path, write operations) for having `readOnlyHint=True`; this is intentional and E2E validated (3x bash_exec(sleep 3) completed in 3.3s vs 9s sequential).
Applied to files:
autogpt_platform/backend/backend/util/workspace.py
📚 Learning: 2026-04-08T17:28:23.439Z
Learnt from: CR
Repo: Significant-Gravitas/AutoGPT PR: 0
File: autogpt_platform/backend/AGENTS.md:0-0
Timestamp: 2026-04-08T17:28:23.439Z
Learning: Applies to autogpt_platform/backend/**/*.py : Be aware of TOCTOU (Time-Of-Check-Time-Of-Use) issues — avoid check-then-act patterns for file access and credit charging
Applied to files:
autogpt_platform/backend/backend/util/workspace.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/util/workspace.py
📚 Learning: 2026-04-01T04:17:38.279Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12632
File: autogpt_platform/backend/backend/copilot/sdk/tool_adapter.py:530-535
Timestamp: 2026-04-01T04:17:38.279Z
Learning: In `autogpt_platform/backend/backend/copilot/sdk/tool_adapter.py`, the `ToolAnnotations(readOnlyHint=True)` annotation (stored as `_PARALLEL_ANNOTATION`) is intentionally applied to ALL registered MCP tools — including E2B write/edit tools (e.g., `write_file`, `edit_file`). This is a parallel-dispatch hint to the Claude Agent SDK CLI, not a semantic read-only contract. The `_READ_ONLY_E2B_TOOLS` set was dead code and was removed in commit `12ae03c`; the constant was renamed from `_READONLY_ANNOTATION` to `_PARALLEL_ANNOTATION` in commit `c88ca88` to avoid confusion. Do not flag this as a correctness issue.
Applied to files:
autogpt_platform/backend/backend/util/workspace.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/util/workspace.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/util/workspace.pyautogpt_platform/backend/backend/copilot/rate_limit_test.pyautogpt_platform/backend/backend/util/workspace_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/util/workspace.pyautogpt_platform/backend/backend/copilot/rate_limit_test.pyautogpt_platform/backend/backend/util/workspace_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/util/workspace.pyautogpt_platform/backend/backend/copilot/rate_limit_test.pyautogpt_platform/backend/backend/util/workspace_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/util/workspace.pyautogpt_platform/backend/backend/copilot/rate_limit_test.pyautogpt_platform/backend/backend/util/workspace_test.py
📚 Learning: 2026-03-15T23:39:39.754Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12385
File: autogpt_platform/backend/backend/copilot/rate_limit.py:0-0
Timestamp: 2026-03-15T23:39:39.754Z
Learning: In `autogpt_platform/backend/backend/copilot/rate_limit.py`, `record_token_usage` uses the same helper functions (`_daily_reset_time()` / `_weekly_reset_time()`) to compute both `resets_at` (the reset timestamp returned to callers) and the Redis key `expire` seconds. This single-source-of-truth design guarantees that the reported reset times and the actual Redis TTLs are always in sync — there is no separate TTL constant that could diverge from the calendar-boundary calculation.
Applied to files:
autogpt_platform/backend/backend/copilot/rate_limit_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/rate_limit_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/rate_limit_test.py
📚 Learning: 2026-04-08T17:28:23.439Z
Learnt from: CR
Repo: Significant-Gravitas/AutoGPT PR: 0
File: autogpt_platform/backend/AGENTS.md:0-0
Timestamp: 2026-04-08T17:28:23.439Z
Learning: Applies to autogpt_platform/backend/**/*_test.py : Mock at boundaries — mock where the symbol is **used**, not where it's **defined**; after refactoring, update mock targets to match new module paths
Applied to files:
autogpt_platform/backend/backend/util/workspace_test.py
📚 Learning: 2026-04-08T17:28:23.439Z
Learnt from: CR
Repo: Significant-Gravitas/AutoGPT PR: 0
File: autogpt_platform/backend/AGENTS.md:0-0
Timestamp: 2026-04-08T17:28:23.439Z
Learning: Applies to autogpt_platform/backend/**/*_test.py : When creating snapshots in tests, use `poetry run pytest path/to/test.py --snapshot-update`; always review snapshot changes with `git diff` before committing
Applied to files:
autogpt_platform/backend/backend/util/workspace_test.py
📚 Learning: 2026-03-19T15:10:53.815Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12483
File: autogpt_platform/backend/backend/copilot/tools/test_dry_run.py:298-303
Timestamp: 2026-03-19T15:10:53.815Z
Learning: In Python unittest.mock, the correct patch target depends on whether an import is eager (module-level) or lazy (inside a function/branch):
- **Module-level import** (`from foo.bar import baz` at top of file): patch where the name is used, e.g. `patch("mymodule.baz")`.
- **Lazy import** (`from foo.bar import baz` inside a function/branch, executed at call time): patch the source module, e.g. `patch("foo.bar.baz")`, because the fresh `from ... import` at call time will look up the (now-patched) name in the source module's dict.
This pattern appears in `autogpt_platform/backend/backend/copilot/tools/helpers.py` where `simulate_block` is lazily imported inside the `if dry_run:` block, making `patch("backend.executor.simulator.simulate_block")` the correct target in tests.
Applied to files:
autogpt_platform/backend/backend/util/workspace_test.py
- Move duplicate-path check before virus scan for non-overwrite writes (avoids slow scan on already-doomed requests) - Add VirusScanError logging in CoPilot WriteWorkspaceFileTool so infrastructure failures (ClamAV outage) are logged at ERROR level Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
autogpt_platform/backend/backend/util/workspace.py (1)
202-202: Consider moving workspace-size summation to a DB aggregate on the write hot path.Line 202 calls
get_workspace_total_size(...)for every write, and that accessor currently fetches all file rows then sums in Python. With high file counts, this adds avoidable read/transfer overhead before every upload. A DB-sideSUM(sizeBytes)(filtered on non-deleted files) would scale better.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@autogpt_platform/backend/backend/util/workspace.py` at line 202, The current write path calls get_workspace_total_size(self.workspace_id) which loads all file rows and sums in Python; change get_workspace_total_size (or create a new DB-backed helper used on the write hot path) to perform a database aggregate (e.g., SELECT SUM(size_bytes) FROM files WHERE workspace_id = ? AND deleted = false) and return that value instead of fetching all rows, then have the upload/write logic continue to call this aggregate helper so per-write cost is a single DB SUM rather than transferring all file records.
🤖 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/tools/workspace_files.py`:
- Around line 897-907: The current except ValueError block contains an
unreachable isinstance(e, VirusScanError) check because VirusScanError (from
backend.api.features.store.exceptions) inherits MediaUploadError, not
ValueError; fix by importing VirusScanError at module top and add an explicit
except VirusScanError: handler before the except ValueError block to log
infrastructure errors via logger.error using an f-string (e.g.,
logger.error(f"Virus scan infrastructure error: {e})", exc_info=True)) and
return the same ErrorResponse(session_id=session_id) flow; also update the other
logger.error("Error writing workspace file: %s", e, exc_info=True) to use an
f-string for consistency.
---
Nitpick comments:
In `@autogpt_platform/backend/backend/util/workspace.py`:
- Line 202: The current write path calls
get_workspace_total_size(self.workspace_id) which loads all file rows and sums
in Python; change get_workspace_total_size (or create a new DB-backed helper
used on the write hot path) to perform a database aggregate (e.g., SELECT
SUM(size_bytes) FROM files WHERE workspace_id = ? AND deleted = false) and
return that value instead of fetching all rows, then have the upload/write logic
continue to call this aggregate helper so per-write cost is a single DB SUM
rather than transferring all file records.
🪄 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: 1822c04e-2ef7-45d0-8478-c0f2bc0085d5
📒 Files selected for processing (2)
autogpt_platform/backend/backend/copilot/tools/workspace_files.pyautogpt_platform/backend/backend/util/workspace.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). (11)
- GitHub Check: check API types
- GitHub Check: Seer Code Review
- GitHub Check: Cursor Bugbot
- GitHub Check: type-check (3.13)
- GitHub Check: type-check (3.11)
- GitHub Check: test (3.12)
- GitHub Check: test (3.11)
- GitHub Check: test (3.13)
- GitHub Check: end-to-end tests
- GitHub Check: Check PR Status
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (2)
autogpt_platform/backend/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
autogpt_platform/backend/**/*.py: Use Python 3.11 (required; managed by Poetry via pyproject.toml) for backend development
Always run 'poetry run format' (Black + isort) before linting in backend development
Always run 'poetry run lint' (ruff) after formatting in backend development
autogpt_platform/backend/**/*.py: Usepoetry run ...command for executing Python package dependencies
Use top-level imports only — avoid local/inner imports except for lazy imports of heavy optional dependencies likeopenpyxl
Use absolute imports withfrom backend.module import ...for cross-package imports; single-dot relative imports are acceptable for sibling modules within the same package; avoid double-dot relative imports
Do not use duck typing — avoidhasattr/getattr/isinstancefor type dispatch; use typed interfaces/unions/protocols instead
Use Pydantic models over dataclass/namedtuple/dict for structured data
Do not use linter suppressors — no# type: ignore,# noqa,# pyright: ignore; fix the type/code instead
Prefer list comprehensions over manual loop-and-append patterns
Use early return with guard clauses first to avoid deep nesting
Use%sfor deferred interpolation indebuglog statements for efficiency; use f-strings elsewhere for readability (e.g.,logger.debug("Processing %s items", count)vslogger.info(f"Processing {count} items"))
Sanitize error paths by usingos.path.basename()in error messages to avoid leaking directory structure
Be aware of TOCTOU (Time-Of-Check-Time-Of-Use) issues — avoid check-then-act patterns for file access and credit charging
Usetransaction=Truefor Redis pipelines to ensure atomicity on multi-step operations
Usemax(0, value)guards for computed values that should never be negative
Keep files under ~300 lines; if a file grows beyond this, split by responsibility (extract helpers, models, or a sub-module into a new file)
Keep functions under ~40 lines; extract named helpers when a function grows longer
...
Files:
autogpt_platform/backend/backend/copilot/tools/workspace_files.pyautogpt_platform/backend/backend/util/workspace.py
autogpt_platform/{backend,autogpt_libs}/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Format Python code with
poetry run format
Files:
autogpt_platform/backend/backend/copilot/tools/workspace_files.pyautogpt_platform/backend/backend/util/workspace.py
🧠 Learnings (30)
📓 Common learnings
Learnt from: ntindle
Repo: Significant-Gravitas/AutoGPT PR: 12780
File: autogpt_platform/backend/backend/util/workspace.py:198-221
Timestamp: 2026-04-15T02:06:38.113Z
Learning: In `autogpt_platform/backend/backend/util/workspace.py`, the TOCTOU gap between the `get_workspace_total_size()` pre-check and the subsequent storage write + DB insert in `WorkspaceManager.write_file()` is an accepted trade-off. Reasons: (1) the workspace is single-user-scoped, so a true race requires precise concurrent timing from the same user; (2) the REST upload route (`autogpt_platform/backend/backend/api/features/workspace/routes.py`) already has a post-write quota check with soft-delete rollback as a safety net for that path; (3) adding the same rollback inside `write_file()` would couple the manager to HTTP semantics. The pre-write check catches the overwhelming majority of cases. Do NOT flag this as a blocking TOCTOU issue.
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: 12773
File: autogpt_platform/backend/backend/copilot/pending_messages.py:52-64
Timestamp: 2026-04-14T14:36:22.396Z
Learning: In `autogpt_platform/backend/backend/copilot` (PR `#12773`, commit d7bced0c6): when draining pending messages into `session.messages`, each message's text is sanitized via `strip_user_context_tags` before persistence to prevent user-controlled `<user_context>` injection from bypassing the trusted server-side context prefix. Additionally, if `upsert_chat_session` fails after draining, the drained `PendingMessage` objects are requeued back to Redis to avoid silent message loss. Do NOT flag the drain-then-requeue pattern as redundant — it is the intentional failure-resilience strategy for the pending buffer.
📚 Learning: 2026-04-15T02:06:38.113Z
Learnt from: ntindle
Repo: Significant-Gravitas/AutoGPT PR: 12780
File: autogpt_platform/backend/backend/util/workspace.py:198-221
Timestamp: 2026-04-15T02:06:38.113Z
Learning: In `autogpt_platform/backend/backend/util/workspace.py`, the TOCTOU gap between the `get_workspace_total_size()` pre-check and the subsequent storage write + DB insert in `WorkspaceManager.write_file()` is an accepted trade-off. Reasons: (1) the workspace is single-user-scoped, so a true race requires precise concurrent timing from the same user; (2) the REST upload route (`autogpt_platform/backend/backend/api/features/workspace/routes.py`) already has a post-write quota check with soft-delete rollback as a safety net for that path; (3) adding the same rollback inside `write_file()` would couple the manager to HTTP semantics. The pre-write check catches the overwhelming majority of cases. Do NOT flag this as a blocking TOCTOU issue.
Applied to files:
autogpt_platform/backend/backend/copilot/tools/workspace_files.pyautogpt_platform/backend/backend/util/workspace.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/workspace_files.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/workspace_files.pyautogpt_platform/backend/backend/util/workspace.py
📚 Learning: 2026-04-11T23:51:20.769Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12750
File: autogpt_platform/backend/backend/copilot/sdk/file_tools.py:0-0
Timestamp: 2026-04-11T23:51:20.769Z
Learning: In `autogpt_platform/backend/backend/copilot/sdk/file_tools.py`, the TOCTOU gap between `is_allowed_local_path(resolved, sdk_cwd)` and the subsequent `open()` in `_handle_write_non_e2b` (and analogous read/edit handlers) is an accepted trade-off. In non-E2B mode the SDK working directory is ephemeral and user-scoped (created per session under `/tmp`), making the practical attack surface minimal. The E2B sandbox already uses `readlink -f` for defense-in-depth. Additional TOCTOU hardening (e.g., `O_NOFOLLOW`) should be reconsidered only if/when persistent workspaces are introduced. Do not flag this as a blocking issue for non-E2B ephemeral workspace paths.
Applied to files:
autogpt_platform/backend/backend/copilot/tools/workspace_files.pyautogpt_platform/backend/backend/util/workspace.py
📚 Learning: 2026-04-03T13:53:33.653Z
Learnt from: Pwuts
Repo: Significant-Gravitas/AutoGPT PR: 12206
File: autogpt_platform/backend/snapshots/v2_unhandled_exception_500:1-5
Timestamp: 2026-04-03T13:53:33.653Z
Learning: In Significant-Gravitas/AutoGPT (autogpt_platform), the catch-all `Exception` handler in `autogpt_platform/backend/backend/api/utils/exceptions.py` (`_handle_error()`) intentionally surfaces `str(exc)` as the `detail` field in HTTP 500 responses for non-Prisma errors. This is by design: errors are logged server-side, and the detail helps API consumers report issues. Only `PrismaError` responses are sanitized (see commit ce6910b4a). Do not flag `str(exc)` in the generic 500 handler as an information disclosure issue; the snapshot `autogpt_platform/backend/snapshots/v2_unhandled_exception_500` ("connection refused") correctly reflects this behavior.
Applied to files:
autogpt_platform/backend/backend/copilot/tools/workspace_files.py
📚 Learning: 2026-03-16T16:30:30.764Z
Learnt from: Abhi1992002
Repo: Significant-Gravitas/AutoGPT PR: 12417
File: autogpt_platform/backend/backend/blocks/agent_mail/pods.py:62-74
Timestamp: 2026-03-16T16:30:30.764Z
Learning: In autogpt_platform/backend/backend/blocks/**/*.py, explicit try/except in the `run()` method is NOT required for standard error handling. The block framework's `_execute()` method in `_base.py` catches unhandled exceptions and re-raises them as `BlockExecutionError` or `BlockUnknownError`. Additionally, when a block yields `("error", message)`, `_execute()` immediately raises `BlockExecutionError` — so the `error` output port never propagates downstream. Explicit try/except is only needed when partial output must be controlled (e.g., attachment blocks that must skip yielding `content_base64` on failure).
Applied to files:
autogpt_platform/backend/backend/copilot/tools/workspace_files.py
📚 Learning: 2026-03-16T16:30:20.657Z
Learnt from: Abhi1992002
Repo: Significant-Gravitas/AutoGPT PR: 12417
File: autogpt_platform/backend/backend/blocks/agent_mail/threads.py:80-102
Timestamp: 2026-03-16T16:30:20.657Z
Learning: In autogpt_platform/backend/backend/blocks/agent_mail/ (and other blocks under autogpt_platform/backend/backend/blocks/), the block executor framework (backend/executor/manager.py lines ~708-733) automatically catches all uncaught exceptions from a block's `run()` method and emits them on the `"error"` output. Explicit try/except blocks within `run()` are therefore not required for standard error propagation — they are only needed when partial output behaviour must be controlled (e.g., preventing some outputs from being yielded on failure, as in attachment blocks). This is the standard pattern across the codebase.
Applied to files:
autogpt_platform/backend/backend/copilot/tools/workspace_files.py
📚 Learning: 2026-03-16T16:32:29.430Z
Learnt from: Abhi1992002
Repo: Significant-Gravitas/AutoGPT PR: 12417
File: autogpt_platform/backend/backend/blocks/agent_mail/pods.py:62-74
Timestamp: 2026-03-16T16:32:29.430Z
Learning: In autogpt_platform/backend/backend/blocks/**/*.py, the Block base class `execute()` method in `backend/blocks/_base.py` already wraps `run()` in a try/except that converts uncaught exceptions into `BlockExecutionError`/`BlockUnknownError`. Therefore, explicit try/except in individual block `run()` methods is redundant and not the established pattern (e.g., Gmail, Slack, Todoist blocks omit it). Exception: blocks like attachment blocks that need to distinguish between success and error yield paths within the generator use explicit try/except for branching control, not for the framework's error routing.
Applied to files:
autogpt_platform/backend/backend/copilot/tools/workspace_files.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/tools/workspace_files.py
📚 Learning: 2026-03-17T07:24:34.302Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12385
File: autogpt_platform/backend/backend/copilot/rate_limit.py:0-0
Timestamp: 2026-03-17T07:24:34.302Z
Learning: In `autogpt_platform/backend/backend/copilot/rate_limit.py`, all fail-open `except` blocks catch `(RedisError, ConnectionError, OSError)` specifically — not bare `except Exception`. This applies to `_session_reset_from_ttl`, `get_usage_status`, `check_rate_limit`, and `record_token_usage`. The narrowed tuple ensures only genuine Redis/network failures are swallowed; unexpected exceptions propagate normally.
Applied to files:
autogpt_platform/backend/backend/copilot/tools/workspace_files.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/workspace_files.pyautogpt_platform/backend/backend/util/workspace.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/workspace_files.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/workspace_files.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/workspace_files.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/workspace_files.pyautogpt_platform/backend/backend/util/workspace.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/workspace_files.pyautogpt_platform/backend/backend/util/workspace.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/workspace_files.pyautogpt_platform/backend/backend/util/workspace.py
📚 Learning: 2026-04-01T04:17:43.495Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12632
File: autogpt_platform/backend/backend/copilot/tools/workspace_files.py:0-0
Timestamp: 2026-04-01T04:17:43.495Z
Learning: In autogpt_platform/backend/backend/copilot/sdk/tool_adapter.py (PR `#12632`, commit 12ae03c), the per-tool `BaseTool.read_only` property approach was removed. Instead, `readOnlyHint=True` (via `ToolAnnotations`) is applied unconditionally to ALL tools — including side-effect tools like `bash_exec` and `write_workspace_file` — to enable fully parallel dispatch by the Anthropic SDK/CLI. Do not flag tools with mutating operations (e.g. save_to_path, write operations) for having `readOnlyHint=True`; this is intentional and E2E validated (3x bash_exec(sleep 3) completed in 3.3s vs 9s sequential).
Applied to files:
autogpt_platform/backend/backend/util/workspace.py
📚 Learning: 2026-04-08T17:28:23.439Z
Learnt from: CR
Repo: Significant-Gravitas/AutoGPT PR: 0
File: autogpt_platform/backend/AGENTS.md:0-0
Timestamp: 2026-04-08T17:28:23.439Z
Learning: Applies to autogpt_platform/backend/**/*.py : Be aware of TOCTOU (Time-Of-Check-Time-Of-Use) issues — avoid check-then-act patterns for file access and credit charging
Applied to files:
autogpt_platform/backend/backend/util/workspace.py
📚 Learning: 2026-04-01T04:17:38.279Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12632
File: autogpt_platform/backend/backend/copilot/sdk/tool_adapter.py:530-535
Timestamp: 2026-04-01T04:17:38.279Z
Learning: In `autogpt_platform/backend/backend/copilot/sdk/tool_adapter.py`, the `ToolAnnotations(readOnlyHint=True)` annotation (stored as `_PARALLEL_ANNOTATION`) is intentionally applied to ALL registered MCP tools — including E2B write/edit tools (e.g., `write_file`, `edit_file`). This is a parallel-dispatch hint to the Claude Agent SDK CLI, not a semantic read-only contract. The `_READ_ONLY_E2B_TOOLS` set was dead code and was removed in commit `12ae03c`; the constant was renamed from `_READONLY_ANNOTATION` to `_PARALLEL_ANNOTATION` in commit `c88ca88` to avoid confusion. Do not flag this as a correctness issue.
Applied to files:
autogpt_platform/backend/backend/util/workspace.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/util/workspace.py
📚 Learning: 2026-04-08T17:26:41.549Z
Learnt from: CR
Repo: Significant-Gravitas/AutoGPT PR: 0
File: classic/CLAUDE.md:0-0
Timestamp: 2026-04-08T17:26:41.549Z
Learning: Applies to classic/.autogpt/autogpt.yaml : Workspace-level permissions in {workspace}/.autogpt/autogpt.yaml should use pattern syntax: command_name(glob_pattern) with support for {workspace}, **, and * tokens
Applied to files:
autogpt_platform/backend/backend/util/workspace.py
📚 Learning: 2026-03-23T06:36:25.447Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12440
File: autogpt_platform/frontend/src/app/(platform)/library/components/LibraryImportWorkflowDialog/useLibraryImportWorkflowDialog.ts:0-0
Timestamp: 2026-03-23T06:36:25.447Z
Learning: In Significant-Gravitas/AutoGPT PR `#12440`, the `LibraryImportWorkflowDialog` (previously `LibraryImportCompetitorDialog`) and its associated generated API hook (`usePostV2ImportACompetitorWorkflowN8nMakeComZapier` / `usePostV2ImportAWorkflowFromAnotherToolN8nMakeComZapier`) were removed in a subsequent refactor. Workflow import from external platforms (n8n, Make.com, Zapier) now uses a server action `fetchWorkflowFromUrl` instead of direct API calls or generated orval hooks. Do not expect or flag missing generated hook usage for workflow import in `autogpt_platform/frontend/src/app/(platform)/library/components/LibraryImportWorkflowDialog/`.
Applied to files:
autogpt_platform/backend/backend/util/workspace.py
📚 Learning: 2026-03-12T14:42:40.552Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12385
File: autogpt_platform/backend/backend/copilot/rate_limit.py:141-170
Timestamp: 2026-03-12T14:42:40.552Z
Learning: In Significant-Gravitas/AutoGPT, `check_rate_limit` in `autogpt_platform/backend/backend/copilot/rate_limit.py` is intentionally a "pre-turn soft check" (not a hard atomic reservation). Because LLM token counts are unknown before generation completes, a strict check-and-reserve is impractical. The TOCTOU race (two concurrent turns both passing the pre-check and both committing via `record_token_usage`) is an accepted trade-off. If stricter enforcement is ever needed, the approach is a Lua script doing GET+INCRBY atomically in Redis.
Applied to files:
autogpt_platform/backend/backend/util/workspace.py
📚 Learning: 2026-04-07T18:08:03.548Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12701
File: .claude/skills/orchestrate/scripts/verify-complete.sh:120-121
Timestamp: 2026-04-07T18:08:03.548Z
Learning: In Significant-Gravitas/AutoGPT, verify-complete.sh (`.claude/skills/orchestrate/scripts/verify-complete.sh`) uses `commits[-1].committedDate` (not `updatedAt`) to identify stale CHANGES_REQUESTED reviews. This is intentional: `updatedAt` changes on any PR activity (bot comments, label changes, description edits), which would falsely classify a reviewer's CHANGES_REQUESTED as stale — a silent false negative. The `committedDate` edge case (commit created locally before a review but pushed after) only causes a false positive (unnecessary re-brief), which is the safer failure mode. Do not suggest switching to `updatedAt` for this comparison.
Applied to files:
autogpt_platform/backend/backend/util/workspace.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/util/workspace.py
📚 Learning: 2026-04-14T06:33:59.422Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12774
File: autogpt_platform/backend/backend/copilot/tools/e2b_sandbox.py:0-0
Timestamp: 2026-04-14T06:33:59.422Z
Learning: In `autogpt_platform/backend/backend/copilot/tools/e2b_sandbox.py`, the `asyncio.wait_for()` retry loop around `AsyncSandbox.create()` (introduced in PR `#12774`) can leak up to `_SANDBOX_CREATE_MAX_RETRIES - 1` (≤2) orphaned E2B sandboxes per hang incident because `wait_for` cancels only the client-side wait while E2B may complete server-side provisioning. With the default `on_timeout="pause"` lifecycle, leaked orphaned sandboxes are **paused** (not killed) when their original `end_at` is reached and persist indefinitely until explicitly killed — there is NO automatic E2B project-level cleanup. Operators must manage these manually or via their own cleanup jobs. The sandbox_id is not accessible from the timed-out coroutine, so recovery via `AsyncSandbox.connect(sandbox_id)` is not possible at timeout. This is an intentionally accepted trade-off; a proper fix is deferred to a follow-up PR. Do NOT flag the retry loop as a blocking issue.
Applied to files:
autogpt_platform/backend/backend/util/workspace.py
📚 Learning: 2026-04-03T13:50:29.037Z
Learnt from: Pwuts
Repo: Significant-Gravitas/AutoGPT PR: 12206
File: autogpt_platform/backend/backend/api/external/v2/rate_limit.py:24-56
Timestamp: 2026-04-03T13:50:29.037Z
Learning: In `autogpt_platform/backend/backend/api/external/v2/rate_limit.py`, the `RateLimiter` class uses in-process (per-worker) memory for sliding-window rate limiting. This is intentionally documented as a known limitation via WARNING comments in the module and class docstrings. A full Redis-backed migration (using ZADD/ZREMRANGEBYSCORE/ZCARD with TTL/Lua for atomic multi-replica enforcement) is deferred to a later PR. Do not re-flag the in-memory implementation as a blocking bug — the limitation is documented and accepted for the initial v2 external API release.
Applied to files:
autogpt_platform/backend/backend/util/workspace.py
📚 Learning: 2026-04-07T09:54:36.517Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12691
File: .claude/skills/orchestrate/scripts/poll-cycle.sh:29-30
Timestamp: 2026-04-07T09:54:36.517Z
Learning: In Significant-Gravitas/AutoGPT, `.claude/skills/orchestrate/scripts/poll-cycle.sh` uses an atomic `.tmp` + `mv` write pattern for `~/.claude/orchestrator-state.json`. The lack of `flock`-based serialization across concurrent writers (poll-cycle.sh, add, stop) is an accepted trade-off because the CronCreate polling interval is ~2 minutes, making true races low-probability. Using `flock` on the state file plus per-writer `mktemp` temp paths is noted as a future improvement, not a current requirement.
Applied to files:
autogpt_platform/backend/backend/util/workspace.py
🔇 Additional comments (1)
autogpt_platform/backend/backend/util/workspace.py (1)
223-235: Good sequencing: duplicate-path check before scan, with scanning centralized here.This keeps duplicate failures deterministic/cheap and preserves one scanning authority for persisted writes.
Add file storage bar to the CoPilot usage limits popover showing used/limit bytes, percentage, and file count. Fetches from the existing GET /workspace/storage/usage endpoint. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
🤖 Picking this up from Discord — code wiring is solid, all the bug-bot/CodeRabbit threads are addressed. My concerns are the user-facing model, which the per-tier numbers alone don't settle. 1. What happens when a user runs out?REST upload → 413 with Failure scenario: AutoPilot is mid-task, agent calls 2. Has product signed off on the tier numbers?
3. Storage semantics — persistent or windowed?Token rate limits reset daily/weekly. Storage in this PR is a lifetime hard quota — once you're full, you stay full until you manually delete. 4. Retention / GC?
5. (Tech) 80% warning is per-write, not per-user
|
…le raises The 80% warning fired on every write inside the band, producing repeated log entries during a single CoPilot turn that writes multiple files. The frontend storage bar already conveys usage to the user, and ops alerting belongs on a metric, not a log line that scales with write rate. Also document `VirusDetectedError` / `VirusScanError` in `WorkspaceManager.write_file` so callers can decide between explicit handling and generic `Exception` catches.
|
🤖 Follow-up on my review:
|
majdyz
left a comment
majdyz
left a comment
There was a problem hiding this comment.
🤖 Approved.
What I changed on this branch
- Dropped the per-write 80%
logger.warninginWorkspaceManager.write_file— the FE storage bar already conveys this; logs scaling with write rate isn't the right channel. - Documented
VirusDetectedError/VirusScanErrorin thewrite_filedocstring (Sentry r3164688581) and resolved the thread. The broader "callers swallow the event" claim doesn't apply —_save_browser_stateand_persist_and_summarizelog withexc_info=True(the exception type is preserved), andutil/file.store_media_filealready scans beforewrite_file.
Open product items — non-blocking, please file follow-up tickets
#1 — Out-of-storage UX. REST returns 413, CoPilot returns ErrorResponse, FE relays the string. AutoPilot already has delete_workspace_file and the REST DELETE /files/{file_id} — the missing piece is a "storage is full, want me to clean up?" affordance at the upload site (CoPilot tool prompt, FE banner) so users don't have to leave the chat to recover.
#3 — Persistent vs windowed semantics. Confirmed offline that lifetime hard quota is the intended model (vs daily/weekly reset like token limits). That's fine in isolation, but coupled with #4 below it means hitting the cap once → manual recovery loop forever, with no self-healing.
#4 — Retention / GC. soft_delete_workspace_file renames the path and flips isDeleted; the storage-backend blob is never reaped. No scheduled cleanup of soft-deleted files or of inactive/cancelled workspaces — permanent S3/disk bill on churned accounts. Even a basic "soft-deleted > N days → hard delete + storage GC" job would prevent the unbounded growth.
(Item #2 — BASIC=250 MB tier ladder — confirmed intentional; #5 — 80% warning — fixed in this branch.)
github-project-automation
Bot
moved this from 🚧 Needs work
to 👍🏼 Mergeable
in AutoGPT development kanban
…OU rollback blob leak WriteWorkspaceFileTool: when write_file rejects with "Storage limit exceeded", append a recovery hint pointing at list_workspace_files + delete_workspace_file. The agent reads this on the next turn and can offer to clean up before falling back to "ask the user to upgrade", so the user doesn't have to leave the chat. Upload route TOCTOU rollback: route the over-quota cleanup through WorkspaceManager.delete_file instead of soft_delete_workspace_file. The latter only flips isDeleted on the DB row and renames the path — the storage backend blob would survive, leaking storage on every concurrent-upload race. delete_file removes the blob first, then soft-deletes the DB record.
majdyz
left a comment
majdyz
left a comment
There was a problem hiding this comment.
🤖 Re-approving on c93dac9.
Two more fixes pushed
#1 — storage-full self-recovery hint (workspace_files.py). When WriteWorkspaceFileTool catches "Storage limit exceeded", the ErrorResponse now appends a hint pointing the agent at list_workspace_files + delete_workspace_file. So when storage fills mid-conversation, the agent reads the hint on the next turn and can offer to clean up rather than just relaying the error — user doesn't have to leave the chat to recover. Test in workspace_files_test.py updated to assert the hint is present.
#4 (the immediate leak) — TOCTOU rollback no longer orphans storage blobs (routes.py). Replaced the direct soft_delete_workspace_file call in the post-write quota rollback with manager.delete_file(...). The manager removes the storage blob first, then soft-deletes the DB row — same semantics for the DB but no orphaned blob on every concurrent-upload race. test_upload_post_write_quota_race updated to assert manager.delete_file is called (not soft_delete_workspace_file). Verified locally — passes.
Still open (separate follow-up PR)
Tier-based file retention via an expiresAt column (cleaner than a periodic GC: get_workspace_total_size filters by expiresAt > NOW() OR NULL, so quota self-heals as files age out). Schema change + migration + cleanup-job-for-blobs — bigger scope, doesn't belong on this PR. I'll open a separate ticket/PR for it.
(Items #2 and #5 already settled in earlier rounds; #3 is resolved by retention landing in the follow-up.)
ntindle
dismissed stale reviews from autogpt-pr-reviewer-in-dev[bot] and autogpt-pr-reviewer[bot]
old
PR #12900 moved local dev to a 3-node Redis Cluster on ports 17000-17002 but didn't update .env.default, so anyone running the backend outside Docker gets "cannot connect to redis cluster". Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 6e4354a. Configure here.
github-project-automation
Bot
moved this from 👍🏼 Mergeable
to ✅ Done
in AutoGPT development kanban
…e-limit through DB-manager (#12992) ## Why Two production fixes surfaced from John Ababseh's dev testing on 2026-05-01 (Discord thread `1499923303609925793`): - **Issue #5** — chat session `c93dc51f-bb38-4427-975a-6dc033358689` finished after multiple minutes of work and showed only `(Done — no further commentary.)` Langfuse trace `7d1a674eb7c84ffb5a4b34875306eea9` shows the model wrote the entire restaurant-list answer **inside an extended-thinking `ThinkingBlock`** (931 completion tokens, $0.50 spend) and ended the turn with empty `content: []`. Our existing thinking-only guard immediately stamped the placeholder, so the user never saw the actual answer the model already generated. - **Issue #2** — every image-generation request (`AIImageCustomizerBlock` / `AIImageGeneratorBlock`) on dev failed with `prisma.errors.ClientNotConnectedError: Client is not connected to the query engine`. Regression from #12780 (tier-based workspace file storage limits): the new pre-write quota check at `util/workspace.py:225` called `get_workspace_total_size` directly from `backend.data.workspace`, which is a Prisma read. The copilot-executor process doesn't connect Prisma — it RPCs into `database-manager` for everything else — so every `manager.write_file()` from a tool blew up. ## What - **Issue 5** — layered fallback for thinking-only final turns: 1. Adapter sets `pending_thinking_only_reprompt` and defers placeholder/StreamFinish. 2. Driver re-enters the SDK loop and fires one synthetic `client.query("Please write a brief user-facing summary of what you found...")`. 3. If the re-prompt also returns thinking-only, promote the most recent `ThinkingBlock` content to a visible `TextDelta`. 4. Only when thinking is also empty, emit the original `(Done — no further commentary.)` placeholder. Bounded to **one** re-prompt per turn so the worst case is ~one extra LLM call. - **Issue 2** — route the storage-limit pre-check through the existing `workspace_db()` accessor and expose `get_workspace_total_size` on `DatabaseManager` so the copilot-executor RPCs into database-manager (where Prisma is connected), the same path other workspace queries on this codepath use. ## How `backend/copilot/sdk/response_adapter.py` - New `pending_thinking_only_reprompt`, `thinking_only_reprompted`, `_last_thinking_content` fields on `SDKResponseAdapter`. - Capture latest `block.thinking` when streaming reasoning so the second-tier promote-fallback has content. - ResultMessage thinking-only branch — first hit defers; second hit prefers `_last_thinking_content`, falls back to placeholder. `backend/copilot/sdk/service.py` - Wrap the `async for sdk_msg in _iter_sdk_messages(client):` block in a `while True:` retry loop. After the inner loop ends, check `pending_thinking_only_reprompt` — if set and not yet retried, fire `client.query(_THINKING_ONLY_REPROMPT, ...)` and re-enter; else break. Most of the diff is +4-space indentation churn. - Module-level `_THINKING_ONLY_REPROMPT` constant for the re-prompt copy. `backend/data/db_manager.py` - Import `get_workspace_total_size` and expose it via `_(...)` so it becomes an RPC on `DatabaseManager` and the corresponding async client. `backend/util/workspace.py` - Drop the direct `get_workspace_total_size` import; call `workspace_db().get_workspace_total_size(self.workspace_id)` instead. `backend/util/workspace_test.py`, `backend/copilot/sdk/response_adapter_test.py` - Existing thinking-only test split into three: defer-on-first-pass, promote-thinking-on-second-pass, fallback-to-placeholder-when-no-thinking. - Updated `test_flush_unresolved_at_result_message` to expect deferral instead of immediate placeholder. - New `test_write_file_storage_check_routes_through_workspace_db_accessor` proving the storage-limit pre-check goes through the accessor (would have caught Issue 2). ## Test plan - [x] `poetry run pytest backend/copilot/sdk/response_adapter_test.py backend/util/workspace_test.py` — 67 pass - [x] `poetry run ruff check` on changed files — clean - [x] `poetry run black` / `poetry run isort` on changed files — clean - [x] `/pr-test --fix` against dev preview to exercise the re-prompt + image-write paths end-to-end - [x] `/pr-polish` until merge-ready ## Related - Regression introduced by #12780 (tier-based workspace file storage limits)
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: majdyz <zamil.majdy@agpt.co>
…e-limit through DB-manager (#12992) ## Why Two production fixes surfaced from John Ababseh's dev testing on 2026-05-01 (Discord thread `1499923303609925793`): - **Issue #5** — chat session `c93dc51f-bb38-4427-975a-6dc033358689` finished after multiple minutes of work and showed only `(Done — no further commentary.)` Langfuse trace `7d1a674eb7c84ffb5a4b34875306eea9` shows the model wrote the entire restaurant-list answer **inside an extended-thinking `ThinkingBlock`** (931 completion tokens, $0.50 spend) and ended the turn with empty `content: []`. Our existing thinking-only guard immediately stamped the placeholder, so the user never saw the actual answer the model already generated. - **Issue #2** — every image-generation request (`AIImageCustomizerBlock` / `AIImageGeneratorBlock`) on dev failed with `prisma.errors.ClientNotConnectedError: Client is not connected to the query engine`. Regression from #12780 (tier-based workspace file storage limits): the new pre-write quota check at `util/workspace.py:225` called `get_workspace_total_size` directly from `backend.data.workspace`, which is a Prisma read. The copilot-executor process doesn't connect Prisma — it RPCs into `database-manager` for everything else — so every `manager.write_file()` from a tool blew up. ## What - **Issue 5** — layered fallback for thinking-only final turns: 1. Adapter sets `pending_thinking_only_reprompt` and defers placeholder/StreamFinish. 2. Driver re-enters the SDK loop and fires one synthetic `client.query("Please write a brief user-facing summary of what you found...")`. 3. If the re-prompt also returns thinking-only, promote the most recent `ThinkingBlock` content to a visible `TextDelta`. 4. Only when thinking is also empty, emit the original `(Done — no further commentary.)` placeholder. Bounded to **one** re-prompt per turn so the worst case is ~one extra LLM call. - **Issue 2** — route the storage-limit pre-check through the existing `workspace_db()` accessor and expose `get_workspace_total_size` on `DatabaseManager` so the copilot-executor RPCs into database-manager (where Prisma is connected), the same path other workspace queries on this codepath use. ## How `backend/copilot/sdk/response_adapter.py` - New `pending_thinking_only_reprompt`, `thinking_only_reprompted`, `_last_thinking_content` fields on `SDKResponseAdapter`. - Capture latest `block.thinking` when streaming reasoning so the second-tier promote-fallback has content. - ResultMessage thinking-only branch — first hit defers; second hit prefers `_last_thinking_content`, falls back to placeholder. `backend/copilot/sdk/service.py` - Wrap the `async for sdk_msg in _iter_sdk_messages(client):` block in a `while True:` retry loop. After the inner loop ends, check `pending_thinking_only_reprompt` — if set and not yet retried, fire `client.query(_THINKING_ONLY_REPROMPT, ...)` and re-enter; else break. Most of the diff is +4-space indentation churn. - Module-level `_THINKING_ONLY_REPROMPT` constant for the re-prompt copy. `backend/data/db_manager.py` - Import `get_workspace_total_size` and expose it via `_(...)` so it becomes an RPC on `DatabaseManager` and the corresponding async client. `backend/util/workspace.py` - Drop the direct `get_workspace_total_size` import; call `workspace_db().get_workspace_total_size(self.workspace_id)` instead. `backend/util/workspace_test.py`, `backend/copilot/sdk/response_adapter_test.py` - Existing thinking-only test split into three: defer-on-first-pass, promote-thinking-on-second-pass, fallback-to-placeholder-when-no-thinking. - Updated `test_flush_unresolved_at_result_message` to expect deferral instead of immediate placeholder. - New `test_write_file_storage_check_routes_through_workspace_db_accessor` proving the storage-limit pre-check goes through the accessor (would have caught Issue 2). ## Test plan - [x] `poetry run pytest backend/copilot/sdk/response_adapter_test.py backend/util/workspace_test.py` — 67 pass - [x] `poetry run ruff check` on changed files — clean - [x] `poetry run black` / `poetry run isort` on changed files — clean - [x] `/pr-test --fix` against dev preview to exercise the re-prompt + image-write paths end-to-end - [x] `/pr-polish` until merge-ready ## Related - Regression introduced by #12780 (tier-based workspace file storage limits)
2 participants
Why / What / How
Why: Storage limits should scale with subscription tier, matching how token rate limits already work. The old flat 500 MB global config applied equally to free and enterprise users. Additionally, CoPilot's
WriteWorkspaceFileToolbypassed the storage quota entirely since enforcement only lived in the REST upload route.What: Replace the global
max_workspace_storage_mbenv-var config with per-tier limits derived from the user'sSubscriptionTier, backed by LaunchDarkly for runtime tuning:How:
_DEFAULT_TIER_WORKSPACE_STORAGE_MBmapping andget_workspace_storage_limit_bytes(user_id)helper inrate_limit.py(alongside existingTIER_MULTIPLIERS).get_workspace_storage_limits_mb()resolver (flag:copilot-tier-workspace-storage-limits) mirroring the chat-limit pattern, so limits can be tuned per-tier without a deploy.WorkspaceManager.write_file()so all callers (REST uploads + CoPilot agent writes) are covered. The REST route keeps a post-write TOCTOU check as a race-condition safety net.NO_TIERentry ensures users who cancel their subscription get a real (smaller) storage cap rather than unlimited storage.Changes 🏗️
copilot/rate_limit.py— Added_DEFAULT_TIER_WORKSPACE_STORAGE_MBdict,_fetch_workspace_storage_limits_flag()LD resolver,get_workspace_storage_limits_mb(), and updatedget_workspace_storage_limit_bytes()to use LD-backed maputil/feature_flag.py— AddedCOPILOT_TIER_WORKSPACE_STORAGE_LIMITSflag enum entryutil/workspace.py— Added tier-based quota check + 80% warning inWorkspaceManager.write_file()api/features/workspace/routes.py— Removed redundant pre-write quota check (now in manager), updated TOCTOU check +get_storage_usage()to use tier-based limit, addedVirusDetectedErrorcatchutil/settings.py— Removedmax_workspace_storage_mbconfig fieldfrontend/UsagePanelContent.tsx— Shows file storage bar viaWorkspaceStorageSectionfrontend/useWorkspaceStorage.ts— New hook for workspace storage datafrontend/direct-upload.ts— Surfaces backend quota/scan error messagesTests 🧪
copilot/rate_limit_test.py— AddedTestGetWorkspaceStorageLimits(5 tests: LD resolution, partial override, bad values skipped, flag disabled fallback, empty dict fallback) + updatedTestWorkspaceStorageLimitswith NO_TIER coveragedata/credit_subscription_test.py— Addedtest_sync_subscription_from_stripe_cancelled_applies_no_tier_storage_limit(verifies unsubscribe downgrades storage limit)util/workspace_test.py— Addedtest_write_file_rejects_upload_when_usage_already_exceeds_downgraded_limitroutes_test.py— Updated mocks to match new architecture (22 tests)frontend/UsagePanelContentRender.test.tsx— Added test for storage section rendering when usage windows are nullChecklist 📋
For code changes:
routes_test.py)workspace_test.py)rate_limit_test.py)credit_subscription_test.py)For configuration changes:
.env.defaultis updated or already compatible with my changes (removed config field was never in.env.default)docker-compose.ymlis updated or already compatible with my changes (no docker changes needed)MAX_WORKSPACE_STORAGE_MBenv var — tier mapping + LD flag is now the source of truthNote
Medium Risk
Changes enforcement of workspace storage quotas and virus-scan error handling across both REST uploads and CoPilot tools, which can affect upload/write flows and user experience. Uses LaunchDarkly-driven tier limits, so misconfiguration or flag parsing issues could unexpectedly block or allow storage usage.
Overview
Introduces tier-based workspace storage quotas (defaulting from 250MB to 15GB by
SubscriptionTier) with optional LaunchDarkly overrides via a newcopilot-tier-workspace-storage-limitsflag.Moves quota enforcement into
WorkspaceManager.write_file()(covering both REST uploads and CoPilot agent writes), including correct accounting for overwrites; REST upload keeps a post-write TOCTOU check but now rolls back viaWorkspaceManager.delete_file()to avoid orphaned blobs.Updates API and tools error mapping: virus-detected returns 400, virus-scan infrastructure failures return 500, quota/file-size map to 413 with more user-friendly messages. Adds a
/storage/usagetier-based limit response, surfaces storage usage in the frontend usage panel, and improves direct-upload error parsing.Reviewed by Cursor Bugbot for commit 6e4354a. Bugbot is set up for automated code reviews on this repo. Configure here.