fix(backend): born-tenanted executions/library/notifications — stop the startup-migration tenancy sweep leak - #13745
Conversation
…he startup-migration tenancy sweep leak New rows for executions, library agents, and notification batches were created without org/team when the caller didn't supply one, landing in the untenanted pool the startup org-migration sweep has to backfill on every boot (~4852 executions/boot). Resolve the user's default org/team at each create funnel so the rows are born tenanted: - add_graph_execution (create path only): fall back to get_user_default_team when organization_id is falsy — covers legacy schedules, sub-graphs inheriting an untenanted parent, and any caller that omits tenancy. Resume/requeue is untouched (it backfills from the persisted row). - add_graph_to_library: stamp organizationId + Team on both the create and the UniqueViolation update branches, mirroring create_library_agent. - create_or_add_to_user_notification_batch: optional org/team params, resolve default team internally when unset, stamp the create branch. get_user_default_team returning (None, None) (bootstrap not yet done) leaves the row untenanted and lets the boot sweep catch it — a run/add/ notification never crashes over tenancy. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NBwmh7CkHiF8vKLBf2GWTU
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:
WalkthroughDefault organization and team resolution now applies to new library entries, notification batches, and graph executions. Explicit tenancy is preserved, while missing or failed defaults leave records untenanted. Re-added library entries retain existing tenancy assignments. ChangesDefault tenancy propagation
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant Library
participant Notifications
participant ExecutionUtils
participant TenancyResolver
participant Storage
Client->>Library: add graph to library
Library->>TenancyResolver: resolve default tenancy
TenancyResolver-->>Library: organization/team or unset
Library->>Storage: persist new library entry
Client->>Notifications: create notification batch
Notifications->>TenancyResolver: resolve when tenancy is omitted
TenancyResolver-->>Notifications: organization/team or unset
Notifications->>Storage: persist batch tenancy
Client->>ExecutionUtils: create graph execution
ExecutionUtils->>TenancyResolver: resolve when tenancy is omitted
TenancyResolver-->>ExecutionUtils: organization/team or unset
ExecutionUtils->>Storage: persist execution tenancy
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
autogpt_platform/backend/backend/api/features/library/_add_to_library.py (1)
104-104: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove this import to module scope.
get_user_default_teamis neither lazy nor optional. Import it at the top of the module; update tests to patchbackend.api.features.library._add_to_library.get_user_default_team, where the symbol will be used. As per coding guidelines, “Use top-level imports only — avoid local/inner imports except for lazy imports of heavy optional dependencies likeopenpyxl.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@autogpt_platform/backend/backend/api/features/library/_add_to_library.py` at line 104, Move the get_user_default_team import to module scope in _add_to_library.py, remove the local import, and update tests to patch backend.api.features.library._add_to_library.get_user_default_team.Source: Coding guidelines
autogpt_platform/backend/backend/data/notifications.py (1)
474-474: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove the new imports to module scope.
These imports are neither heavy nor optional, so they should not be lazy imports.
autogpt_platform/backend/backend/data/notifications.py#L474-L474: importget_user_default_teamat module scope.autogpt_platform/backend/backend/data/notifications_test.py#L472-L472: importAsyncMockat module scope.autogpt_platform/backend/backend/data/notifications_test.py#L505-L505: reuse that module-levelAsyncMockimport.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 current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@autogpt_platform/backend/backend/data/notifications.py` at line 474, Move get_user_default_team to module-level imports in autogpt_platform/backend/backend/data/notifications.py at lines 474-474. In autogpt_platform/backend/backend/data/notifications_test.py, import AsyncMock at module scope at lines 472-472 and reuse that import at lines 505-505, removing the local import while preserving existing test behavior.Source: Coding guidelines
autogpt_platform/backend/backend/executor/utils.py (1)
1303-1306: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse module-level imports and patch the bound dependency.
These imports are neither heavy nor optional. Move them to module scope; then patch the resolver where
add_graph_executionuses it.
autogpt_platform/backend/backend/executor/utils.py#L1303-L1306: importget_user_default_teamat module scope.autogpt_platform/backend/backend/executor/conftest.py#L43-L48: patchbackend.executor.utils.get_user_default_team.autogpt_platform/backend/backend/executor/utils_test.py#L1893-L1899: moveGraphExecutionWithNodesto module scope and update the helper documentation.autogpt_platform/backend/backend/executor/utils_test.py#L1951-L1955: patchbackend.executor.utils.get_user_default_team.autogpt_platform/backend/backend/executor/utils_test.py#L2022-L2026: moveExecutionContextto module scope.As per coding guidelines, use top-level imports only except for lazy imports of heavy optional dependencies.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@autogpt_platform/backend/backend/executor/utils.py` around lines 1303 - 1306, Move get_user_default_team to module scope in autogpt_platform/backend/backend/executor/utils.py:1303-1306, and patch backend.executor.utils.get_user_default_team in autogpt_platform/backend/backend/executor/conftest.py:43-48 and utils_test.py:1951-1955 so add_graph_execution uses the patched dependency. In utils_test.py:1893-1899, move GraphExecutionWithNodes to module scope and update the helper documentation; in utils_test.py:2022-2026, move ExecutionContext to module scope. Keep imports top-level unless they are heavy optional dependencies.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@autogpt_platform/backend/backend/api/features/library/_add_to_library.py`:
- Around line 104-106: Update the default-tenancy lookup around
get_user_default_team in the library-add flow to catch lookup exceptions, log
the failure, and continue with (None, None) so adding to the library is not
aborted. Add a regression test that configures an AsyncMock for
get_user_default_team with a side effect and verifies the fallback behavior.
In `@autogpt_platform/backend/backend/executor/utils.py`:
- Around line 1303-1308: Update the tenancy resolution branch around
organization_id and team_id so get_user_default_team is called only when both
values are unset, preserving any explicitly supplied team_id; alternatively
reject partial tenancy explicitly. Add a test covering team-only input and
verifying that the supplied team_id is retained.
---
Nitpick comments:
In `@autogpt_platform/backend/backend/api/features/library/_add_to_library.py`:
- Line 104: Move the get_user_default_team import to module scope in
_add_to_library.py, remove the local import, and update tests to patch
backend.api.features.library._add_to_library.get_user_default_team.
In `@autogpt_platform/backend/backend/data/notifications.py`:
- Line 474: Move get_user_default_team to module-level imports in
autogpt_platform/backend/backend/data/notifications.py at lines 474-474. In
autogpt_platform/backend/backend/data/notifications_test.py, import AsyncMock at
module scope at lines 472-472 and reuse that import at lines 505-505, removing
the local import while preserving existing test behavior.
In `@autogpt_platform/backend/backend/executor/utils.py`:
- Around line 1303-1306: Move get_user_default_team to module scope in
autogpt_platform/backend/backend/executor/utils.py:1303-1306, and patch
backend.executor.utils.get_user_default_team in
autogpt_platform/backend/backend/executor/conftest.py:43-48 and
utils_test.py:1951-1955 so add_graph_execution uses the patched dependency. In
utils_test.py:1893-1899, move GraphExecutionWithNodes to module scope and update
the helper documentation; in utils_test.py:2022-2026, move ExecutionContext to
module scope. Keep imports top-level unless they are heavy optional
dependencies.
🪄 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 Plus
Run ID: 9bb64acd-ee0f-4fdd-a42d-6c3c75a84e52
📒 Files selected for processing (7)
autogpt_platform/backend/backend/api/features/library/_add_to_library.pyautogpt_platform/backend/backend/api/features/library/_add_to_library_test.pyautogpt_platform/backend/backend/data/notifications.pyautogpt_platform/backend/backend/data/notifications_test.pyautogpt_platform/backend/backend/executor/conftest.pyautogpt_platform/backend/backend/executor/utils.pyautogpt_platform/backend/backend/executor/utils_test.py
📜 Review details
⏰ Context from checks skipped due to timeout. (7)
- GitHub Check: check API types
- GitHub Check: Analyze (python)
- GitHub Check: end-to-end tests
- GitHub Check: Check PR Status
- GitHub Check: test (3.13)
- GitHub Check: test (3.11)
- GitHub Check: test (3.12)
🧰 Additional context used
📓 Path-based instructions (7)
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/executor/utils.pyautogpt_platform/backend/backend/executor/conftest.pyautogpt_platform/backend/backend/api/features/library/_add_to_library.pyautogpt_platform/backend/backend/data/notifications_test.pyautogpt_platform/backend/backend/data/notifications.pyautogpt_platform/backend/backend/executor/utils_test.pyautogpt_platform/backend/backend/api/features/library/_add_to_library_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/executor/utils.pyautogpt_platform/backend/backend/executor/conftest.pyautogpt_platform/backend/backend/api/features/library/_add_to_library.pyautogpt_platform/backend/backend/data/notifications_test.pyautogpt_platform/backend/backend/data/notifications.pyautogpt_platform/backend/backend/executor/utils_test.pyautogpt_platform/backend/backend/api/features/library/_add_to_library_test.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/library/_add_to_library.pyautogpt_platform/backend/backend/api/features/library/_add_to_library_test.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/library/_add_to_library.pyautogpt_platform/backend/backend/api/features/library/_add_to_library_test.py
autogpt_platform/backend/backend/data/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
All data access in backend requires user ID checks; verify this for any 'data/*.py' changes
Files:
autogpt_platform/backend/backend/data/notifications_test.pyautogpt_platform/backend/backend/data/notifications.py
autogpt_platform/**/data/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
For changes touching
data/*.py, validate user ID checks or explain why not needed
Files:
autogpt_platform/backend/backend/data/notifications_test.pyautogpt_platform/backend/backend/data/notifications.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/data/notifications_test.pyautogpt_platform/backend/backend/executor/utils_test.pyautogpt_platform/backend/backend/api/features/library/_add_to_library_test.py
🧠 Learnings (13)
📚 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/executor/utils.pyautogpt_platform/backend/backend/executor/conftest.pyautogpt_platform/backend/backend/api/features/library/_add_to_library.pyautogpt_platform/backend/backend/data/notifications_test.pyautogpt_platform/backend/backend/data/notifications.pyautogpt_platform/backend/backend/executor/utils_test.pyautogpt_platform/backend/backend/api/features/library/_add_to_library_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/executor/utils.pyautogpt_platform/backend/backend/executor/conftest.pyautogpt_platform/backend/backend/api/features/library/_add_to_library.pyautogpt_platform/backend/backend/data/notifications_test.pyautogpt_platform/backend/backend/data/notifications.pyautogpt_platform/backend/backend/executor/utils_test.pyautogpt_platform/backend/backend/api/features/library/_add_to_library_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/executor/utils.pyautogpt_platform/backend/backend/executor/conftest.pyautogpt_platform/backend/backend/api/features/library/_add_to_library.pyautogpt_platform/backend/backend/data/notifications_test.pyautogpt_platform/backend/backend/data/notifications.pyautogpt_platform/backend/backend/executor/utils_test.pyautogpt_platform/backend/backend/api/features/library/_add_to_library_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/executor/utils.pyautogpt_platform/backend/backend/executor/conftest.pyautogpt_platform/backend/backend/api/features/library/_add_to_library.pyautogpt_platform/backend/backend/data/notifications_test.pyautogpt_platform/backend/backend/data/notifications.pyautogpt_platform/backend/backend/executor/utils_test.pyautogpt_platform/backend/backend/api/features/library/_add_to_library_test.py
📚 Learning: 2026-04-15T02:43:36.890Z
Learnt from: ntindle
Repo: Significant-Gravitas/AutoGPT PR: 12780
File: autogpt_platform/backend/backend/copilot/tools/workspace_files.py:0-0
Timestamp: 2026-04-15T02:43:36.890Z
Learning: When reviewing Python exception handlers, do not flag `isinstance(e, X)` checks as dead/unreachable if the caught exception `X` is a subclass of the exception type being handled. For example, if `X` (e.g., `VirusScanError`) inherits from `ValueError` (directly or via an intermediate class) and it can be raised within an `except ValueError:` block, then `isinstance(e, X)` inside that handler is reachable and should not be treated as dead code.
Applied to files:
autogpt_platform/backend/backend/executor/utils.pyautogpt_platform/backend/backend/executor/conftest.pyautogpt_platform/backend/backend/api/features/library/_add_to_library.pyautogpt_platform/backend/backend/data/notifications_test.pyautogpt_platform/backend/backend/data/notifications.pyautogpt_platform/backend/backend/executor/utils_test.pyautogpt_platform/backend/backend/api/features/library/_add_to_library_test.py
📚 Learning: 2026-05-23T05:29:43.085Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 13200
File: autogpt_platform/backend/backend/executor/scheduler.py:590-593
Timestamp: 2026-05-23T05:29:43.085Z
Learning: When reviewing Python code that uses Pydantic discriminated/tagged unions (e.g., `Annotated[Union[...], Field(discriminator="kind")]`), recognize that using `isinstance(x, SomeVariantInfo)` to narrow the union is an intentional and correct runtime guard and should also enable static type narrowing in tools like Pyright. Do not recommend replacing such `isinstance`-based narrowing with `cast(...)` when the check already proves the variant at runtime.
Applied to files:
autogpt_platform/backend/backend/executor/utils.pyautogpt_platform/backend/backend/executor/conftest.pyautogpt_platform/backend/backend/api/features/library/_add_to_library.pyautogpt_platform/backend/backend/data/notifications_test.pyautogpt_platform/backend/backend/data/notifications.pyautogpt_platform/backend/backend/executor/utils_test.pyautogpt_platform/backend/backend/api/features/library/_add_to_library_test.py
📚 Learning: 2026-04-22T11:46:04.431Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12881
File: autogpt_platform/backend/backend/copilot/config.py:0-0
Timestamp: 2026-04-22T11:46:04.431Z
Learning: Do not flag the Claude Sonnet 4.6 model ID as incorrect when it uses the project’s established hyphenated convention: `anthropic/claude-sonnet-4-6`. This hyphen form is the intentional, production convention and should be treated as valid (including in files like llm.py, blocks tests, reasoning.py, `_is_anthropic_model` tests, and config defaults). Note that OpenRouter also accepts the dot variant `anthropic/claude-sonnet-4.6`, so either form may be tolerated, but `anthropic/claude-sonnet-4-6` should be considered the standard to match project usage.
Applied to files:
autogpt_platform/backend/backend/executor/utils.pyautogpt_platform/backend/backend/executor/conftest.pyautogpt_platform/backend/backend/api/features/library/_add_to_library.pyautogpt_platform/backend/backend/data/notifications_test.pyautogpt_platform/backend/backend/data/notifications.pyautogpt_platform/backend/backend/executor/utils_test.pyautogpt_platform/backend/backend/api/features/library/_add_to_library_test.py
📚 Learning: 2026-04-22T11:46:12.892Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12881
File: autogpt_platform/backend/backend/copilot/baseline/service.py:322-332
Timestamp: 2026-04-22T11:46:12.892Z
Learning: In this codebase (Significant-Gravitas/AutoGPT), OpenRouter-routed Anthropic model IDs should use the hyphen-separated convention (e.g., `anthropic/claude-sonnet-4-6`, `anthropic/claude-opus-4-6`). Although OpenRouter may accept both hyphen and dot variants, treat the hyphen-separated form as the intended, correct codebase-wide convention and do not flag it as an error. Only flag the dot-separated variant (e.g., `anthropic/claude-sonnet-4.6`) as incorrect when reviewing/validating model ID strings for OpenRouter-routed Anthropic models.
Applied to files:
autogpt_platform/backend/backend/executor/utils.pyautogpt_platform/backend/backend/executor/conftest.pyautogpt_platform/backend/backend/api/features/library/_add_to_library.pyautogpt_platform/backend/backend/data/notifications_test.pyautogpt_platform/backend/backend/data/notifications.pyautogpt_platform/backend/backend/executor/utils_test.pyautogpt_platform/backend/backend/api/features/library/_add_to_library_test.py
📚 Learning: 2026-05-07T18:48:14.242Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 13040
File: autogpt_platform/backend/backend/blocks/llm.py:0-0
Timestamp: 2026-05-07T18:48:14.242Z
Learning: In this repository, isort may split imports from the same module into separate blocks when some imports are aliased (e.g., `from module import X as Y`) and others are not. Preserve the two-block layout when it results from isort (such as keeping `from openai.types.chat import ChatCompletion as OpenAIChatCompletion` separate from non-aliased imports from `openai.types.chat`). Do not treat that split as a style issue during review; merging them into a single block can fail CI with `Imports are incorrectly sorted and/or formatted`.
Applied to files:
autogpt_platform/backend/backend/executor/utils.pyautogpt_platform/backend/backend/executor/conftest.pyautogpt_platform/backend/backend/api/features/library/_add_to_library.pyautogpt_platform/backend/backend/data/notifications_test.pyautogpt_platform/backend/backend/data/notifications.pyautogpt_platform/backend/backend/executor/utils_test.pyautogpt_platform/backend/backend/api/features/library/_add_to_library_test.py
📚 Learning: 2026-05-26T14:24:34.866Z
Learnt from: Abhi1992002
Repo: Significant-Gravitas/AutoGPT PR: 13217
File: autogpt_platform/backend/backend/api/features/search/service.py:137-137
Timestamp: 2026-05-26T14:24:34.866Z
Learning: In the Significant-Gravitas/AutoGPT backend, treat `user_id` (an opaque UUID used only for correlation/tracing) as non-PII. Do not flag direct logging of `user_id` in `logger.warning`/`logger.info` statements as a PII exposure issue, as the established convention is to log `user_id` for tracing while reserving PII for fields like email or display name.
Applied to files:
autogpt_platform/backend/backend/executor/utils.pyautogpt_platform/backend/backend/executor/conftest.pyautogpt_platform/backend/backend/api/features/library/_add_to_library.pyautogpt_platform/backend/backend/data/notifications_test.pyautogpt_platform/backend/backend/data/notifications.pyautogpt_platform/backend/backend/executor/utils_test.pyautogpt_platform/backend/backend/api/features/library/_add_to_library_test.py
📚 Learning: 2026-06-11T19:39:10.493Z
Learnt from: ntindle
Repo: Significant-Gravitas/AutoGPT PR: 13337
File: autogpt_platform/backend/backend/copilot/graphiti/reranker.py:0-0
Timestamp: 2026-06-11T19:39:10.493Z
Learning: In the Significant-Gravitas/AutoGPT Python backend, when calling the OpenAI Python client `chat.completions.create`, construct the `messages` payload using the concrete typed-dict variants from `openai.types.chat` (e.g., `ChatCompletionSystemMessageParam`, `ChatCompletionUserMessageParam`, etc.) rather than trying to instantiate `ChatCompletionMessageParam` directly. `ChatCompletionMessageParam` is a `Union` alias and is not constructible, so `ChatCompletionMessageParam(role=..., content=...)` should fail type checking. Build each message element with the appropriate concrete typed dict and then annotate the resulting list as `list[ChatCompletionMessageParam]` (e.g., `messages: list[ChatCompletionMessageParam] = [ChatCompletionSystemMessageParam(...), ...]`).
Applied to files:
autogpt_platform/backend/backend/executor/utils.pyautogpt_platform/backend/backend/executor/conftest.pyautogpt_platform/backend/backend/api/features/library/_add_to_library.pyautogpt_platform/backend/backend/data/notifications_test.pyautogpt_platform/backend/backend/data/notifications.pyautogpt_platform/backend/backend/executor/utils_test.pyautogpt_platform/backend/backend/api/features/library/_add_to_library_test.py
📚 Learning: 2026-04-21T04:35:34.710Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12865
File: autogpt_platform/backend/backend/data/credit.py:1584-1584
Timestamp: 2026-04-21T04:35:34.710Z
Learning: When reviewing this codebase, don’t flag snake_case attribute names (e.g., `subscription_tier`, `stripe_customer_id`, `top_up_config`) on the app-layer Pydantic `User` model as “wrong” field names. These are correct for the app-layer model and are expected to be mapped from the Prisma-layer camelCase fields (e.g., `subscriptionTier`, `stripeCustomerId`) inside methods like `User.from_db()`. Only Prisma-returned/raw objects would use camelCase, but functions like `get_user_by_id(user_id: str)` are expected to return the Pydantic app-layer model.
Applied to files:
autogpt_platform/backend/backend/data/notifications_test.pyautogpt_platform/backend/backend/data/notifications.py
📚 Learning: 2026-05-07T15:32:39.703Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 13033
File: autogpt_platform/backend/backend/data/generate_data.py:111-117
Timestamp: 2026-05-07T15:32:39.703Z
Learning: When reviewing the Python data-generation layer, do not treat missing `user_id`/user filtering in calls to graph-metadata resolvers as a security issue if the `graph_id` inputs are already guaranteed to be user-scoped by earlier upstream SQL (e.g., `WHERE "userId" = ...`). In particular, `_resolve_agent_name(graph_id)` in `generate_data.py` correctly calls `get_graph_metadata(graph_id=graph_id)` without a `user_id` parameter by design, because name resolution must also work for user-executed shared/marketplace agents that the user may not own.
Applied to files:
autogpt_platform/backend/backend/data/notifications_test.pyautogpt_platform/backend/backend/data/notifications.py
🔇 Additional comments (5)
autogpt_platform/backend/backend/api/features/library/_add_to_library.py (2)
126-127: LGTM!
149-164: LGTM!autogpt_platform/backend/backend/api/features/library/_add_to_library_test.py (1)
28-72: LGTM!Also applies to: 75-107, 129-163
autogpt_platform/backend/backend/data/notifications.py (1)
458-473: LGTM!Also applies to: 476-504
autogpt_platform/backend/backend/data/notifications_test.py (1)
467-471: LGTM!Also applies to: 474-498, 501-504, 507-532
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev #13745 +/- ##
========================================
Coverage 76.92% 76.93%
========================================
Files 2761 2761
Lines 209521 209758 +237
Branches 20077 20081 +4
========================================
+ Hits 161171 161373 +202
+ Misses 43981 43947 -34
- Partials 4369 4438 +69
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
|
/review |
![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 #13745
PR #13745 — fix(backend): born-tenanted executions/library/notifications — stop the startup-migration tenancy sweep leak
Author: ntindle | Files: 7
🎯 Verdict: APPROVE (with Should-Fix items — no blockers traced)
PR Description Quality
✅ Has Why + What + How — clearly explains the untenanted-row leak (startup sweep re-backfilling a refilling pool), the create-funnel fix, and the guard rails (create-only, explicit-org passthrough, non-blocking (None, None)). Test matrix maps 1:1 to the claims.
What This PR Does
New executions, library agents, and notification batches were being created without an org/team stamp, so a startup migration kept sweeping to backfill a pool that immediately refilled. This PR resolves the caller's own default org/team via get_user_default_team(user_id) at each of the three create funnels (add_graph_execution, add_graph_to_library, create_or_add_to_user_notification_batch) so rows are "born tenanted." Resume/requeue paths are deliberately untouched, explicit tenancy is never overridden, and a (None, None) resolver result leaves the row untenanted rather than crashing the operation.
Specialist Findings
🛡️ Security ✅ — Every resolution path is keyed to the authenticated caller's own user_id (orgs/db.py:147 → _find_personal_org_member, scoped to userId/isOwner/isPersonal); no user-controlled value feeds the org/team lookup, no cross-tenant write or read is possible, no new endpoints/secrets. Worst realistic case is a billing-attribution change to the user's own personal org.
🔵 INFO-level user_id log on the execution hot path (executor/utils.py:1309) — low-sensitivity, prefers %s deferred interpolation.
🏗️ Architecture ✅ — Correct layer for the fix (write-time at the funnel), reuses the existing self-healing resolver, and mirrors create_library_agent. Two structural concerns: data/executor layers now reach up into api.features.orgs.db via call-time imports (masking a circular dependency), and the resolve→stamp idiom is triplicated with divergent shapes (Team.connect vs teamId=).
🟠 Layering inversion (data/notifications.py:471) and hot-path resolver round-trip (executor/utils.py:1300).
⚡ Performance get_user_default_team is uncached and issues 2 sequential DB queries per create. On the execution-create path the fallback fires whenever organization_id is falsy — the common case today — so this adds ~2 round-trips to the system's highest-volume funnel until the untenanted pool drains. Notifications resolve-then-discard on the repeat/UPDATE branch. Indexes are adequate; the fix is caching, not indexing.
🧪 Testing conftest.py fixture. Two of the PR's highest-blast-radius claims are asserted only in prose: ExecutionContext billing propagation (utils_test.py:1938) and the resume/requeue "never re-resolves" guard (no negative assert_not_called test). Library disconnect/untenanted update branches and the notifications (None, None) case are also uncovered.
📖 Quality ✅ — Readability grade A. Clear naming, comments justify the why of each guard rail (permitted by AGENTS.md), faithfully mirrors the established db.py pattern. Only stylistic notes on log verbosity and the repeated tenancy idiom.
📦 Product get_user_default_team returns the default, not the active org), silently overwriting a prior non-default team association with no user feedback.
📬 Discussion REVIEW_REQUIRED): (1) a get_user_default_team exception (vs (None,None)) would propagate and abort the add, and (2) if not organization_id overwrites an explicit team_id when org is falsy but team is supplied.
🔎 QA ✅ — Verified at the DB + unit-test layer where this backend PR lives. A UserNotificationBatch was proven born-tenanted with both org and team in the live DB via the internal resolver; execution and library rows carried org. All 12 PR unit tests pass on independent re-run inside the container; negative auth cases return 401/4xx; no PR-related errors in service logs. Frontend surfaces were blocked by an unrelated plan-selection paywall gate (environment/onboarding artifact, not this diff).
🟠 Should Fix
- Explicit
team_idsilently overwritten when org is falsy (executor/utils.py:1308) — the fallback guard isif not organization_id, so a caller passingteam_idwithoutorganization_idgets its team replaced by the resolved default. Either gate onif not organization_id and not team_id, or reply on the thread confirming no caller passes team-only tenancy, and add a preservation test. (Flagged by: discussion — CodeRabbit Major) - Resolver is not exception-safe on paths that promise "never block" (
_add_to_library.py:106, symmetric innotifications.py:~472,utils.py:~1303) — the guard rails only handle a(None, None)return, not a raised exception, which would propagate and abort the create despite the "never block on tenancy" comment. Wrap in try/except (log + continue untenanted) or confirm on-thread thatget_user_default_teamcannot raise; add aside_effectregression test. (Flagged by: discussion — CodeRabbit Major) - Cache
get_user_default_team(user_id)before it lands on the hot path (executor/utils.py:1303,data/notifications.py:473) — personal org/default team are effectively immutable per user, yet the uncached resolver adds 2 sequential DB queries per create on the system's highest-volume funnel (and resolves-then-discards on the notification UPDATE branch). A per-user TTL/Redis cache collapses this to ~0 reads in steady state. (Flagged by: performance, architect — 2 specialists) - Add the two highest-risk missing tests (
executor/utils_test.py:1938, resume-path negative test) — assert the resolved org/team flows into the constructedExecutionContext(billing), and assertadd_graph_executionon the resume/requeue path callsget_user_default_team.assert_not_called(). Both are cheap at the Prisma boundary already mocked. (Flagged by: testing) - Confirm library re-add team-reset semantics (
_add_to_library.py:146-164) — the UniqueViolation update branch unconditionally re-stamps org and resets/disconnects the team to the user's default, so a plain re-add silently moves a row's tenancy. ConfirmLibraryAgentis strictly a per-user bookmark with no billing/visibility keying off its org tag (add a comment/invariant), or only re-tag when the persisted org actually differs. Add the untesteddisconnect/untenanted update-branch cases. (Flagged by: product, architect, security, testing — 4 specialists)
🟡 Nice to Have
- Consolidate the triplicated resolve→stamp idiom (
utils.py,notifications.py,_add_to_library.py, plus pre-existinglibrary/db.py) into oneresolve_default_tenancyhelper + stamping convention, and relocate the resolver to a shared lower layer so the call-time imports (which mask a circular dependency) can move to module scope. Genuinely out of scope for this fix; track as follow-up debt. (architect, quality, discussion)
🔵 Nits
- Born-tenanted INFO log becomes steady-state noise (
executor/utils.py:1310) — fires on every fallback on the hot path and mixes a literal with an f-string via implicit concatenation. Consider DEBUG (or removal once SECRT-2476 retires the sweep) and collapse to a single%s-style statement. (security, quality, product)
QA Screenshots
| Screenshot | Description |
|---|---|
 |
Authenticated /library redirects to a plan-selection/pricing gate |
Human Review Needed
YES — the change alters which tenant/org owns newly created rows (a trust boundary between tenants and the basis for billing attribution); a maintainer should confirm the re-add team-reset semantics and that no billing/visibility logic keys off the reassigned org tag. Security review found no cross-tenant leak, so this is confirmation, not a red flag.
Risk Assessment
Merge risk: LOW | Rollback: EASY — additive, create-path-only, non-blocking fallback; revert restores prior stamping with the startup sweep still catching untenanted rows.
CI Status
GitHub CI: ✅ 41/41 checks green on head SHA 95cd9dda (tests 3.11/3.12/3.13, type-check, lint, e2e, CodeQL, Snyk, CLA, codecov gates) — per the discussion specialist's fetch; no merge conflicts. Local harness: ✅ all 5 checks pass (frontend lint/types/unit/build; backend poetry run lint).
UI Testing — Variant Results
✅ local: Born-tenanted stamping verified live (notification batch got both org+team; execution+library got org) and all 12 PR unit tests pass on independent re-run; no PR-related errors or regressions.
✅ hosted: Born-tenanted fallback verified live — executions and notification batches are stamped with the user's default org+team on create, explicit tenancy is not overridden, and negative auth/validation cases behave correctly.
…eserve explicit team_id - resolve_default_tenancy(): one shared best-effort wrapper (try/except → (None,None)) used by all three born-tenanted sites, replacing the triplicated inline handling (CodeRabbit + reviewer: duplication, best-effort). - add_graph_execution: only resolve a default when BOTH org and team are unset — never overwrite an explicit team_id (CodeRabbit Major). - tests: team-preservation, per-site lookup-failure, and a resolve_default_tenancy unit test. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Swept both reviewers (CodeRabbit + internal). Dispositions: Fixed
Declined, with reasoning
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
autogpt_platform/backend/backend/data/notifications_test.py (1)
535-564: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd a "team-only" preservation test to match the executor coverage.
The executor path has a dedicated regression test for "explicit
team_idwith noorganization_idmust not be overwritten" (test_add_graph_execution_explicit_team_id_preserved_when_org_absent). This file tests both-explicit and both-absent, but not the team-only case forcreate_or_add_to_user_notification_batch, even though the same guard condition (organization_id is None and team_id is None) governs both.🧪 Suggested test
`@pytest.mark.asyncio` async def test_create_batch_explicit_team_id_preserved_when_org_absent(mocker): mock_upsert = AsyncMock(return_value=object()) mocker.patch( "backend.data.notifications.UserNotificationBatch.prisma" ).return_value.upsert = mock_upsert mocker.patch( "backend.data.notifications.UserNotificationBatchDTO.from_db", return_value="dto-sentinel", ) mock_get_default_team = mocker.patch( "backend.api.features.orgs.db.get_user_default_team", new=AsyncMock(return_value=("fallback-org", "fallback-team")), ) user_id = "notif-team-only-user" await create_or_add_to_user_notification_batch( user_id=user_id, notification_type=NotificationType.AGENT_RUN, notification_data=_make_agent_run_event(user_id), team_id="explicit-team", ) mock_get_default_team.assert_not_called() create_input = mock_upsert.call_args.kwargs["data"]["create"] assert create_input.get("teamId") == "explicit-team" assert create_input.get("organizationId") is None🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@autogpt_platform/backend/backend/data/notifications_test.py` around lines 535 - 564, Add a regression test alongside the existing create_or_add_to_user_notification_batch tests covering an explicit team_id with no organization_id. Mock the default-team lookup, call the function with team_id set, assert the lookup is not called, and verify the upsert create data preserves the explicit teamId while organizationId remains None.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@autogpt_platform/backend/backend/data/notifications_test.py`:
- Around line 535-564: Add a regression test alongside the existing
create_or_add_to_user_notification_batch tests covering an explicit team_id with
no organization_id. Mock the default-team lookup, call the function with team_id
set, assert the lookup is not called, and verify the upsert create data
preserves the explicit teamId while organizationId remains None.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: eb764479-abc2-4d2b-8157-9f6f6bcb6be9
📒 Files selected for processing (8)
autogpt_platform/backend/backend/api/features/library/_add_to_library.pyautogpt_platform/backend/backend/api/features/library/_add_to_library_test.pyautogpt_platform/backend/backend/api/features/orgs/db.pyautogpt_platform/backend/backend/api/features/orgs/routes_test.pyautogpt_platform/backend/backend/data/notifications.pyautogpt_platform/backend/backend/data/notifications_test.pyautogpt_platform/backend/backend/executor/utils.pyautogpt_platform/backend/backend/executor/utils_test.py
📜 Review details
⏰ Context from checks skipped due to timeout. (15)
- GitHub Check: check API types
- GitHub Check: Seer Code Review
- GitHub Check: test (3.11)
- GitHub Check: Check PR Status
- GitHub Check: type-check (3.11)
- GitHub Check: type-check (3.13)
- GitHub Check: test (3.12)
- GitHub Check: lint
- GitHub Check: type-check (3.12)
- GitHub Check: test (3.13)
- GitHub Check: lint
- GitHub Check: types
- GitHub Check: end-to-end tests
- GitHub Check: Analyze (python)
- GitHub Check: Analyze (typescript)
🧰 Additional context used
📓 Path-based instructions (7)
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/orgs/db.pyautogpt_platform/backend/backend/api/features/library/_add_to_library.pyautogpt_platform/backend/backend/executor/utils_test.pyautogpt_platform/backend/backend/executor/utils.pyautogpt_platform/backend/backend/api/features/library/_add_to_library_test.pyautogpt_platform/backend/backend/api/features/orgs/routes_test.pyautogpt_platform/backend/backend/data/notifications_test.pyautogpt_platform/backend/backend/data/notifications.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/orgs/db.pyautogpt_platform/backend/backend/api/features/library/_add_to_library.pyautogpt_platform/backend/backend/api/features/library/_add_to_library_test.pyautogpt_platform/backend/backend/api/features/orgs/routes_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/api/features/orgs/db.pyautogpt_platform/backend/backend/api/features/library/_add_to_library.pyautogpt_platform/backend/backend/executor/utils_test.pyautogpt_platform/backend/backend/executor/utils.pyautogpt_platform/backend/backend/api/features/library/_add_to_library_test.pyautogpt_platform/backend/backend/api/features/orgs/routes_test.pyautogpt_platform/backend/backend/data/notifications_test.pyautogpt_platform/backend/backend/data/notifications.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/orgs/db.pyautogpt_platform/backend/backend/api/features/library/_add_to_library.pyautogpt_platform/backend/backend/api/features/library/_add_to_library_test.pyautogpt_platform/backend/backend/api/features/orgs/routes_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/executor/utils_test.pyautogpt_platform/backend/backend/api/features/library/_add_to_library_test.pyautogpt_platform/backend/backend/api/features/orgs/routes_test.pyautogpt_platform/backend/backend/data/notifications_test.py
autogpt_platform/backend/backend/data/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
All data access in backend requires user ID checks; verify this for any 'data/*.py' changes
Files:
autogpt_platform/backend/backend/data/notifications_test.pyautogpt_platform/backend/backend/data/notifications.py
autogpt_platform/**/data/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
For changes touching
data/*.py, validate user ID checks or explain why not needed
Files:
autogpt_platform/backend/backend/data/notifications_test.pyautogpt_platform/backend/backend/data/notifications.py
🧠 Learnings (13)
📚 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/orgs/db.pyautogpt_platform/backend/backend/api/features/library/_add_to_library.pyautogpt_platform/backend/backend/executor/utils_test.pyautogpt_platform/backend/backend/executor/utils.pyautogpt_platform/backend/backend/api/features/library/_add_to_library_test.pyautogpt_platform/backend/backend/api/features/orgs/routes_test.pyautogpt_platform/backend/backend/data/notifications_test.pyautogpt_platform/backend/backend/data/notifications.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/orgs/db.pyautogpt_platform/backend/backend/api/features/library/_add_to_library.pyautogpt_platform/backend/backend/executor/utils_test.pyautogpt_platform/backend/backend/executor/utils.pyautogpt_platform/backend/backend/api/features/library/_add_to_library_test.pyautogpt_platform/backend/backend/api/features/orgs/routes_test.pyautogpt_platform/backend/backend/data/notifications_test.pyautogpt_platform/backend/backend/data/notifications.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/orgs/db.pyautogpt_platform/backend/backend/api/features/library/_add_to_library.pyautogpt_platform/backend/backend/executor/utils_test.pyautogpt_platform/backend/backend/executor/utils.pyautogpt_platform/backend/backend/api/features/library/_add_to_library_test.pyautogpt_platform/backend/backend/api/features/orgs/routes_test.pyautogpt_platform/backend/backend/data/notifications_test.pyautogpt_platform/backend/backend/data/notifications.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/orgs/db.pyautogpt_platform/backend/backend/api/features/library/_add_to_library.pyautogpt_platform/backend/backend/executor/utils_test.pyautogpt_platform/backend/backend/executor/utils.pyautogpt_platform/backend/backend/api/features/library/_add_to_library_test.pyautogpt_platform/backend/backend/api/features/orgs/routes_test.pyautogpt_platform/backend/backend/data/notifications_test.pyautogpt_platform/backend/backend/data/notifications.py
📚 Learning: 2026-04-15T02:43:36.890Z
Learnt from: ntindle
Repo: Significant-Gravitas/AutoGPT PR: 12780
File: autogpt_platform/backend/backend/copilot/tools/workspace_files.py:0-0
Timestamp: 2026-04-15T02:43:36.890Z
Learning: When reviewing Python exception handlers, do not flag `isinstance(e, X)` checks as dead/unreachable if the caught exception `X` is a subclass of the exception type being handled. For example, if `X` (e.g., `VirusScanError`) inherits from `ValueError` (directly or via an intermediate class) and it can be raised within an `except ValueError:` block, then `isinstance(e, X)` inside that handler is reachable and should not be treated as dead code.
Applied to files:
autogpt_platform/backend/backend/api/features/orgs/db.pyautogpt_platform/backend/backend/api/features/library/_add_to_library.pyautogpt_platform/backend/backend/executor/utils_test.pyautogpt_platform/backend/backend/executor/utils.pyautogpt_platform/backend/backend/api/features/library/_add_to_library_test.pyautogpt_platform/backend/backend/api/features/orgs/routes_test.pyautogpt_platform/backend/backend/data/notifications_test.pyautogpt_platform/backend/backend/data/notifications.py
📚 Learning: 2026-05-23T05:29:43.085Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 13200
File: autogpt_platform/backend/backend/executor/scheduler.py:590-593
Timestamp: 2026-05-23T05:29:43.085Z
Learning: When reviewing Python code that uses Pydantic discriminated/tagged unions (e.g., `Annotated[Union[...], Field(discriminator="kind")]`), recognize that using `isinstance(x, SomeVariantInfo)` to narrow the union is an intentional and correct runtime guard and should also enable static type narrowing in tools like Pyright. Do not recommend replacing such `isinstance`-based narrowing with `cast(...)` when the check already proves the variant at runtime.
Applied to files:
autogpt_platform/backend/backend/api/features/orgs/db.pyautogpt_platform/backend/backend/api/features/library/_add_to_library.pyautogpt_platform/backend/backend/executor/utils_test.pyautogpt_platform/backend/backend/executor/utils.pyautogpt_platform/backend/backend/api/features/library/_add_to_library_test.pyautogpt_platform/backend/backend/api/features/orgs/routes_test.pyautogpt_platform/backend/backend/data/notifications_test.pyautogpt_platform/backend/backend/data/notifications.py
📚 Learning: 2026-04-22T11:46:04.431Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12881
File: autogpt_platform/backend/backend/copilot/config.py:0-0
Timestamp: 2026-04-22T11:46:04.431Z
Learning: Do not flag the Claude Sonnet 4.6 model ID as incorrect when it uses the project’s established hyphenated convention: `anthropic/claude-sonnet-4-6`. This hyphen form is the intentional, production convention and should be treated as valid (including in files like llm.py, blocks tests, reasoning.py, `_is_anthropic_model` tests, and config defaults). Note that OpenRouter also accepts the dot variant `anthropic/claude-sonnet-4.6`, so either form may be tolerated, but `anthropic/claude-sonnet-4-6` should be considered the standard to match project usage.
Applied to files:
autogpt_platform/backend/backend/api/features/orgs/db.pyautogpt_platform/backend/backend/api/features/library/_add_to_library.pyautogpt_platform/backend/backend/executor/utils_test.pyautogpt_platform/backend/backend/executor/utils.pyautogpt_platform/backend/backend/api/features/library/_add_to_library_test.pyautogpt_platform/backend/backend/api/features/orgs/routes_test.pyautogpt_platform/backend/backend/data/notifications_test.pyautogpt_platform/backend/backend/data/notifications.py
📚 Learning: 2026-04-22T11:46:12.892Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12881
File: autogpt_platform/backend/backend/copilot/baseline/service.py:322-332
Timestamp: 2026-04-22T11:46:12.892Z
Learning: In this codebase (Significant-Gravitas/AutoGPT), OpenRouter-routed Anthropic model IDs should use the hyphen-separated convention (e.g., `anthropic/claude-sonnet-4-6`, `anthropic/claude-opus-4-6`). Although OpenRouter may accept both hyphen and dot variants, treat the hyphen-separated form as the intended, correct codebase-wide convention and do not flag it as an error. Only flag the dot-separated variant (e.g., `anthropic/claude-sonnet-4.6`) as incorrect when reviewing/validating model ID strings for OpenRouter-routed Anthropic models.
Applied to files:
autogpt_platform/backend/backend/api/features/orgs/db.pyautogpt_platform/backend/backend/api/features/library/_add_to_library.pyautogpt_platform/backend/backend/executor/utils_test.pyautogpt_platform/backend/backend/executor/utils.pyautogpt_platform/backend/backend/api/features/library/_add_to_library_test.pyautogpt_platform/backend/backend/api/features/orgs/routes_test.pyautogpt_platform/backend/backend/data/notifications_test.pyautogpt_platform/backend/backend/data/notifications.py
📚 Learning: 2026-05-07T18:48:14.242Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 13040
File: autogpt_platform/backend/backend/blocks/llm.py:0-0
Timestamp: 2026-05-07T18:48:14.242Z
Learning: In this repository, isort may split imports from the same module into separate blocks when some imports are aliased (e.g., `from module import X as Y`) and others are not. Preserve the two-block layout when it results from isort (such as keeping `from openai.types.chat import ChatCompletion as OpenAIChatCompletion` separate from non-aliased imports from `openai.types.chat`). Do not treat that split as a style issue during review; merging them into a single block can fail CI with `Imports are incorrectly sorted and/or formatted`.
Applied to files:
autogpt_platform/backend/backend/api/features/orgs/db.pyautogpt_platform/backend/backend/api/features/library/_add_to_library.pyautogpt_platform/backend/backend/executor/utils_test.pyautogpt_platform/backend/backend/executor/utils.pyautogpt_platform/backend/backend/api/features/library/_add_to_library_test.pyautogpt_platform/backend/backend/api/features/orgs/routes_test.pyautogpt_platform/backend/backend/data/notifications_test.pyautogpt_platform/backend/backend/data/notifications.py
📚 Learning: 2026-05-26T14:24:34.866Z
Learnt from: Abhi1992002
Repo: Significant-Gravitas/AutoGPT PR: 13217
File: autogpt_platform/backend/backend/api/features/search/service.py:137-137
Timestamp: 2026-05-26T14:24:34.866Z
Learning: In the Significant-Gravitas/AutoGPT backend, treat `user_id` (an opaque UUID used only for correlation/tracing) as non-PII. Do not flag direct logging of `user_id` in `logger.warning`/`logger.info` statements as a PII exposure issue, as the established convention is to log `user_id` for tracing while reserving PII for fields like email or display name.
Applied to files:
autogpt_platform/backend/backend/api/features/orgs/db.pyautogpt_platform/backend/backend/api/features/library/_add_to_library.pyautogpt_platform/backend/backend/executor/utils_test.pyautogpt_platform/backend/backend/executor/utils.pyautogpt_platform/backend/backend/api/features/library/_add_to_library_test.pyautogpt_platform/backend/backend/api/features/orgs/routes_test.pyautogpt_platform/backend/backend/data/notifications_test.pyautogpt_platform/backend/backend/data/notifications.py
📚 Learning: 2026-06-11T19:39:10.493Z
Learnt from: ntindle
Repo: Significant-Gravitas/AutoGPT PR: 13337
File: autogpt_platform/backend/backend/copilot/graphiti/reranker.py:0-0
Timestamp: 2026-06-11T19:39:10.493Z
Learning: In the Significant-Gravitas/AutoGPT Python backend, when calling the OpenAI Python client `chat.completions.create`, construct the `messages` payload using the concrete typed-dict variants from `openai.types.chat` (e.g., `ChatCompletionSystemMessageParam`, `ChatCompletionUserMessageParam`, etc.) rather than trying to instantiate `ChatCompletionMessageParam` directly. `ChatCompletionMessageParam` is a `Union` alias and is not constructible, so `ChatCompletionMessageParam(role=..., content=...)` should fail type checking. Build each message element with the appropriate concrete typed dict and then annotate the resulting list as `list[ChatCompletionMessageParam]` (e.g., `messages: list[ChatCompletionMessageParam] = [ChatCompletionSystemMessageParam(...), ...]`).
Applied to files:
autogpt_platform/backend/backend/api/features/orgs/db.pyautogpt_platform/backend/backend/api/features/library/_add_to_library.pyautogpt_platform/backend/backend/executor/utils_test.pyautogpt_platform/backend/backend/executor/utils.pyautogpt_platform/backend/backend/api/features/library/_add_to_library_test.pyautogpt_platform/backend/backend/api/features/orgs/routes_test.pyautogpt_platform/backend/backend/data/notifications_test.pyautogpt_platform/backend/backend/data/notifications.py
📚 Learning: 2026-04-21T04:35:34.710Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12865
File: autogpt_platform/backend/backend/data/credit.py:1584-1584
Timestamp: 2026-04-21T04:35:34.710Z
Learning: When reviewing this codebase, don’t flag snake_case attribute names (e.g., `subscription_tier`, `stripe_customer_id`, `top_up_config`) on the app-layer Pydantic `User` model as “wrong” field names. These are correct for the app-layer model and are expected to be mapped from the Prisma-layer camelCase fields (e.g., `subscriptionTier`, `stripeCustomerId`) inside methods like `User.from_db()`. Only Prisma-returned/raw objects would use camelCase, but functions like `get_user_by_id(user_id: str)` are expected to return the Pydantic app-layer model.
Applied to files:
autogpt_platform/backend/backend/data/notifications_test.pyautogpt_platform/backend/backend/data/notifications.py
📚 Learning: 2026-05-07T15:32:39.703Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 13033
File: autogpt_platform/backend/backend/data/generate_data.py:111-117
Timestamp: 2026-05-07T15:32:39.703Z
Learning: When reviewing the Python data-generation layer, do not treat missing `user_id`/user filtering in calls to graph-metadata resolvers as a security issue if the `graph_id` inputs are already guaranteed to be user-scoped by earlier upstream SQL (e.g., `WHERE "userId" = ...`). In particular, `_resolve_agent_name(graph_id)` in `generate_data.py` correctly calls `get_graph_metadata(graph_id=graph_id)` without a `user_id` parameter by design, because name resolution must also work for user-executed shared/marketplace agents that the user may not own.
Applied to files:
autogpt_platform/backend/backend/data/notifications_test.pyautogpt_platform/backend/backend/data/notifications.py
🔇 Additional comments (8)
autogpt_platform/backend/backend/api/features/library/_add_to_library.py (2)
150-165: Re-tagging on unique-violation update — previously flagged, confirmed intentional.This still unconditionally overwrites
organizationId/Team(including disconnecting an existing team) whenever a default org resolves for the re-adding user, which was raised in earlier review rounds. Per the PR discussion this is intentional (per-user bookmark semantics), so no action needed here.
99-131: LGTM!autogpt_platform/backend/backend/executor/utils_test.py (1)
2018-2062: 🗄️ Data Integrity & Integration | ⚡ Quick winVerify resume/requeue-path safety invariant has regression coverage.
A previous review thread asked for a test proving
add_graph_executionnever calls the default-tenancy lookup on the resume/requeue path (graph_exec_idsupplied), since the source comment states this branch "never reaches" the fallback and must not re-tenant an existing row. The tests added here (explicit-team preserved, lookup-failure fallback) only cover the CREATE path — I don't see one covering thegraph_exec_id-supplied branch.#!/bin/bash # Check whether a test exercises add_graph_execution's resume/requeue path # together with an assertion that the default-team lookup is NOT invoked. rg -n "graph_exec_id=" autogpt_platform/backend/backend/executor/utils_test.py -B 3 -A 15 | rg -n "get_user_default_team|assert_not_called"If no such test exists, consider adding one asserting
get_user_default_team.assert_not_called()when resuming viagraph_exec_id.autogpt_platform/backend/backend/api/features/orgs/db.py (1)
176-195: LGTM!autogpt_platform/backend/backend/api/features/orgs/routes_test.py (1)
2946-2972: LGTM!autogpt_platform/backend/backend/api/features/library/_add_to_library_test.py (1)
110-145: LGTM!autogpt_platform/backend/backend/data/notifications.py (1)
454-536: LGTM!autogpt_platform/backend/backend/executor/utils.py (1)
1285-1311: LGTM!
…th executor path) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Added |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
autogpt_platform/backend/backend/data/notifications_test.py (1)
572-572: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove
AsyncMockto the module-level imports.As per coding guidelines, backend Python files should use top-level imports and reserve local imports for lazy heavy optional dependencies such as
openpyxl.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@autogpt_platform/backend/backend/data/notifications_test.py` at line 572, Move the AsyncMock import from its local scope to the module-level imports in notifications_test.py, keeping its existing usage unchanged and following the file’s established import organization.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@autogpt_platform/backend/backend/data/notifications_test.py`:
- Line 572: Move the AsyncMock import from its local scope to the module-level
imports in notifications_test.py, keeping its existing usage unchanged and
following the file’s established import organization.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a6a228dc-0105-4ebf-9c6a-02ce61caf214
📒 Files selected for processing (1)
autogpt_platform/backend/backend/data/notifications_test.py
📜 Review details
⏰ Context from checks skipped due to timeout. (13)
- GitHub Check: check API types
- GitHub Check: test (3.11)
- GitHub Check: test (3.13)
- GitHub Check: test (3.12)
- GitHub Check: type-check (3.13)
- GitHub Check: type-check (3.11)
- GitHub Check: type-check (3.12)
- GitHub Check: lint
- GitHub Check: Seer Code Review
- GitHub Check: Analyze (python)
- GitHub Check: end-to-end tests
- GitHub Check: Check PR Status
- GitHub Check: Analyze (typescript)
🧰 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/data/notifications_test.py
autogpt_platform/backend/backend/data/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
All data access in backend requires user ID checks; verify this for any 'data/*.py' changes
Files:
autogpt_platform/backend/backend/data/notifications_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/data/notifications_test.py
autogpt_platform/**/data/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
For changes touching
data/*.py, validate user ID checks or explain why not needed
Files:
autogpt_platform/backend/backend/data/notifications_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/data/notifications_test.py
🧠 Learnings (13)
📚 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/data/notifications_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/data/notifications_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/data/notifications_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/data/notifications_test.py
📚 Learning: 2026-04-15T02:43:36.890Z
Learnt from: ntindle
Repo: Significant-Gravitas/AutoGPT PR: 12780
File: autogpt_platform/backend/backend/copilot/tools/workspace_files.py:0-0
Timestamp: 2026-04-15T02:43:36.890Z
Learning: When reviewing Python exception handlers, do not flag `isinstance(e, X)` checks as dead/unreachable if the caught exception `X` is a subclass of the exception type being handled. For example, if `X` (e.g., `VirusScanError`) inherits from `ValueError` (directly or via an intermediate class) and it can be raised within an `except ValueError:` block, then `isinstance(e, X)` inside that handler is reachable and should not be treated as dead code.
Applied to files:
autogpt_platform/backend/backend/data/notifications_test.py
📚 Learning: 2026-05-23T05:29:43.085Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 13200
File: autogpt_platform/backend/backend/executor/scheduler.py:590-593
Timestamp: 2026-05-23T05:29:43.085Z
Learning: When reviewing Python code that uses Pydantic discriminated/tagged unions (e.g., `Annotated[Union[...], Field(discriminator="kind")]`), recognize that using `isinstance(x, SomeVariantInfo)` to narrow the union is an intentional and correct runtime guard and should also enable static type narrowing in tools like Pyright. Do not recommend replacing such `isinstance`-based narrowing with `cast(...)` when the check already proves the variant at runtime.
Applied to files:
autogpt_platform/backend/backend/data/notifications_test.py
📚 Learning: 2026-04-21T04:35:34.710Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12865
File: autogpt_platform/backend/backend/data/credit.py:1584-1584
Timestamp: 2026-04-21T04:35:34.710Z
Learning: When reviewing this codebase, don’t flag snake_case attribute names (e.g., `subscription_tier`, `stripe_customer_id`, `top_up_config`) on the app-layer Pydantic `User` model as “wrong” field names. These are correct for the app-layer model and are expected to be mapped from the Prisma-layer camelCase fields (e.g., `subscriptionTier`, `stripeCustomerId`) inside methods like `User.from_db()`. Only Prisma-returned/raw objects would use camelCase, but functions like `get_user_by_id(user_id: str)` are expected to return the Pydantic app-layer model.
Applied to files:
autogpt_platform/backend/backend/data/notifications_test.py
📚 Learning: 2026-05-07T15:32:39.703Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 13033
File: autogpt_platform/backend/backend/data/generate_data.py:111-117
Timestamp: 2026-05-07T15:32:39.703Z
Learning: When reviewing the Python data-generation layer, do not treat missing `user_id`/user filtering in calls to graph-metadata resolvers as a security issue if the `graph_id` inputs are already guaranteed to be user-scoped by earlier upstream SQL (e.g., `WHERE "userId" = ...`). In particular, `_resolve_agent_name(graph_id)` in `generate_data.py` correctly calls `get_graph_metadata(graph_id=graph_id)` without a `user_id` parameter by design, because name resolution must also work for user-executed shared/marketplace agents that the user may not own.
Applied to files:
autogpt_platform/backend/backend/data/notifications_test.py
📚 Learning: 2026-04-22T11:46:04.431Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12881
File: autogpt_platform/backend/backend/copilot/config.py:0-0
Timestamp: 2026-04-22T11:46:04.431Z
Learning: Do not flag the Claude Sonnet 4.6 model ID as incorrect when it uses the project’s established hyphenated convention: `anthropic/claude-sonnet-4-6`. This hyphen form is the intentional, production convention and should be treated as valid (including in files like llm.py, blocks tests, reasoning.py, `_is_anthropic_model` tests, and config defaults). Note that OpenRouter also accepts the dot variant `anthropic/claude-sonnet-4.6`, so either form may be tolerated, but `anthropic/claude-sonnet-4-6` should be considered the standard to match project usage.
Applied to files:
autogpt_platform/backend/backend/data/notifications_test.py
📚 Learning: 2026-04-22T11:46:12.892Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12881
File: autogpt_platform/backend/backend/copilot/baseline/service.py:322-332
Timestamp: 2026-04-22T11:46:12.892Z
Learning: In this codebase (Significant-Gravitas/AutoGPT), OpenRouter-routed Anthropic model IDs should use the hyphen-separated convention (e.g., `anthropic/claude-sonnet-4-6`, `anthropic/claude-opus-4-6`). Although OpenRouter may accept both hyphen and dot variants, treat the hyphen-separated form as the intended, correct codebase-wide convention and do not flag it as an error. Only flag the dot-separated variant (e.g., `anthropic/claude-sonnet-4.6`) as incorrect when reviewing/validating model ID strings for OpenRouter-routed Anthropic models.
Applied to files:
autogpt_platform/backend/backend/data/notifications_test.py
📚 Learning: 2026-05-07T18:48:14.242Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 13040
File: autogpt_platform/backend/backend/blocks/llm.py:0-0
Timestamp: 2026-05-07T18:48:14.242Z
Learning: In this repository, isort may split imports from the same module into separate blocks when some imports are aliased (e.g., `from module import X as Y`) and others are not. Preserve the two-block layout when it results from isort (such as keeping `from openai.types.chat import ChatCompletion as OpenAIChatCompletion` separate from non-aliased imports from `openai.types.chat`). Do not treat that split as a style issue during review; merging them into a single block can fail CI with `Imports are incorrectly sorted and/or formatted`.
Applied to files:
autogpt_platform/backend/backend/data/notifications_test.py
📚 Learning: 2026-05-26T14:24:34.866Z
Learnt from: Abhi1992002
Repo: Significant-Gravitas/AutoGPT PR: 13217
File: autogpt_platform/backend/backend/api/features/search/service.py:137-137
Timestamp: 2026-05-26T14:24:34.866Z
Learning: In the Significant-Gravitas/AutoGPT backend, treat `user_id` (an opaque UUID used only for correlation/tracing) as non-PII. Do not flag direct logging of `user_id` in `logger.warning`/`logger.info` statements as a PII exposure issue, as the established convention is to log `user_id` for tracing while reserving PII for fields like email or display name.
Applied to files:
autogpt_platform/backend/backend/data/notifications_test.py
📚 Learning: 2026-06-11T19:39:10.493Z
Learnt from: ntindle
Repo: Significant-Gravitas/AutoGPT PR: 13337
File: autogpt_platform/backend/backend/copilot/graphiti/reranker.py:0-0
Timestamp: 2026-06-11T19:39:10.493Z
Learning: In the Significant-Gravitas/AutoGPT Python backend, when calling the OpenAI Python client `chat.completions.create`, construct the `messages` payload using the concrete typed-dict variants from `openai.types.chat` (e.g., `ChatCompletionSystemMessageParam`, `ChatCompletionUserMessageParam`, etc.) rather than trying to instantiate `ChatCompletionMessageParam` directly. `ChatCompletionMessageParam` is a `Union` alias and is not constructible, so `ChatCompletionMessageParam(role=..., content=...)` should fail type checking. Build each message element with the appropriate concrete typed dict and then annotate the resulting list as `list[ChatCompletionMessageParam]` (e.g., `messages: list[ChatCompletionMessageParam] = [ChatCompletionSystemMessageParam(...), ...]`).
Applied to files:
autogpt_platform/backend/backend/data/notifications_test.py
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
… AsyncMock import - _add_to_library UniqueViolation branch no longer stamps organizationId/Team: an existing bookmark keeps its tenancy (only the create branch born-tenants). Fixes a team-disconnect when re-adding for a user whose org has no default team (Sentry MEDIUM). Test updated to assert no re-tag. - notifications_test: AsyncMock moved to module-level imports (backend top-level-import guideline). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Fixed (Sentry · Medium — team disconnect on re-add): the UniqueViolation update branch no longer touches Also (CodeRabbit trivial): hoisted the |
There was a problem hiding this comment.
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/library/_add_to_library.py (1)
105-105: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winMove
resolve_default_tenancyto module scope.This is a normal backend dependency, not a lazy heavy optional import.
Proposed fix
+from backend.api.features.orgs.db import resolve_default_tenancy + ... - from backend.api.features.orgs.db import resolve_default_tenancy - organization_id, team_id = await resolve_default_tenancy(user_id)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 current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@autogpt_platform/backend/backend/api/features/library/_add_to_library.py` at line 105, Move the resolve_default_tenancy import from inside its calling function to module scope alongside the other imports in _add_to_library.py. Keep the existing usage unchanged and do not retain a local import.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@autogpt_platform/backend/backend/api/features/library/_add_to_library.py`:
- Line 105: Move the resolve_default_tenancy import from inside its calling
function to module scope alongside the other imports in _add_to_library.py. Keep
the existing usage unchanged and do not retain a local import.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 3661830c-8c31-4b2a-b471-8b6d09a84cb8
📒 Files selected for processing (3)
autogpt_platform/backend/backend/api/features/library/_add_to_library.pyautogpt_platform/backend/backend/api/features/library/_add_to_library_test.pyautogpt_platform/backend/backend/data/notifications_test.py
📜 Review details
⏰ Context from checks skipped due to timeout. (9)
- GitHub Check: check API types
- GitHub Check: test (3.11)
- GitHub Check: type-check (3.11)
- GitHub Check: test (3.12)
- GitHub Check: test (3.13)
- GitHub Check: Seer Code Review
- GitHub Check: end-to-end tests
- GitHub Check: Analyze (python)
- GitHub Check: Check PR Status
🧰 Additional context used
📓 Path-based instructions (7)
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/library/_add_to_library_test.pyautogpt_platform/backend/backend/data/notifications_test.pyautogpt_platform/backend/backend/api/features/library/_add_to_library.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/library/_add_to_library_test.pyautogpt_platform/backend/backend/api/features/library/_add_to_library.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/library/_add_to_library_test.pyautogpt_platform/backend/backend/data/notifications_test.pyautogpt_platform/backend/backend/api/features/library/_add_to_library.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/library/_add_to_library_test.pyautogpt_platform/backend/backend/api/features/library/_add_to_library.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/library/_add_to_library_test.pyautogpt_platform/backend/backend/data/notifications_test.py
autogpt_platform/backend/backend/data/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
All data access in backend requires user ID checks; verify this for any 'data/*.py' changes
Files:
autogpt_platform/backend/backend/data/notifications_test.py
autogpt_platform/**/data/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
For changes touching
data/*.py, validate user ID checks or explain why not needed
Files:
autogpt_platform/backend/backend/data/notifications_test.py
🧠 Learnings (13)
📚 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/library/_add_to_library_test.pyautogpt_platform/backend/backend/data/notifications_test.pyautogpt_platform/backend/backend/api/features/library/_add_to_library.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/library/_add_to_library_test.pyautogpt_platform/backend/backend/data/notifications_test.pyautogpt_platform/backend/backend/api/features/library/_add_to_library.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/library/_add_to_library_test.pyautogpt_platform/backend/backend/data/notifications_test.pyautogpt_platform/backend/backend/api/features/library/_add_to_library.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/library/_add_to_library_test.pyautogpt_platform/backend/backend/data/notifications_test.pyautogpt_platform/backend/backend/api/features/library/_add_to_library.py
📚 Learning: 2026-04-15T02:43:36.890Z
Learnt from: ntindle
Repo: Significant-Gravitas/AutoGPT PR: 12780
File: autogpt_platform/backend/backend/copilot/tools/workspace_files.py:0-0
Timestamp: 2026-04-15T02:43:36.890Z
Learning: When reviewing Python exception handlers, do not flag `isinstance(e, X)` checks as dead/unreachable if the caught exception `X` is a subclass of the exception type being handled. For example, if `X` (e.g., `VirusScanError`) inherits from `ValueError` (directly or via an intermediate class) and it can be raised within an `except ValueError:` block, then `isinstance(e, X)` inside that handler is reachable and should not be treated as dead code.
Applied to files:
autogpt_platform/backend/backend/api/features/library/_add_to_library_test.pyautogpt_platform/backend/backend/data/notifications_test.pyautogpt_platform/backend/backend/api/features/library/_add_to_library.py
📚 Learning: 2026-05-23T05:29:43.085Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 13200
File: autogpt_platform/backend/backend/executor/scheduler.py:590-593
Timestamp: 2026-05-23T05:29:43.085Z
Learning: When reviewing Python code that uses Pydantic discriminated/tagged unions (e.g., `Annotated[Union[...], Field(discriminator="kind")]`), recognize that using `isinstance(x, SomeVariantInfo)` to narrow the union is an intentional and correct runtime guard and should also enable static type narrowing in tools like Pyright. Do not recommend replacing such `isinstance`-based narrowing with `cast(...)` when the check already proves the variant at runtime.
Applied to files:
autogpt_platform/backend/backend/api/features/library/_add_to_library_test.pyautogpt_platform/backend/backend/data/notifications_test.pyautogpt_platform/backend/backend/api/features/library/_add_to_library.py
📚 Learning: 2026-04-22T11:46:04.431Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12881
File: autogpt_platform/backend/backend/copilot/config.py:0-0
Timestamp: 2026-04-22T11:46:04.431Z
Learning: Do not flag the Claude Sonnet 4.6 model ID as incorrect when it uses the project’s established hyphenated convention: `anthropic/claude-sonnet-4-6`. This hyphen form is the intentional, production convention and should be treated as valid (including in files like llm.py, blocks tests, reasoning.py, `_is_anthropic_model` tests, and config defaults). Note that OpenRouter also accepts the dot variant `anthropic/claude-sonnet-4.6`, so either form may be tolerated, but `anthropic/claude-sonnet-4-6` should be considered the standard to match project usage.
Applied to files:
autogpt_platform/backend/backend/api/features/library/_add_to_library_test.pyautogpt_platform/backend/backend/data/notifications_test.pyautogpt_platform/backend/backend/api/features/library/_add_to_library.py
📚 Learning: 2026-04-22T11:46:12.892Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12881
File: autogpt_platform/backend/backend/copilot/baseline/service.py:322-332
Timestamp: 2026-04-22T11:46:12.892Z
Learning: In this codebase (Significant-Gravitas/AutoGPT), OpenRouter-routed Anthropic model IDs should use the hyphen-separated convention (e.g., `anthropic/claude-sonnet-4-6`, `anthropic/claude-opus-4-6`). Although OpenRouter may accept both hyphen and dot variants, treat the hyphen-separated form as the intended, correct codebase-wide convention and do not flag it as an error. Only flag the dot-separated variant (e.g., `anthropic/claude-sonnet-4.6`) as incorrect when reviewing/validating model ID strings for OpenRouter-routed Anthropic models.
Applied to files:
autogpt_platform/backend/backend/api/features/library/_add_to_library_test.pyautogpt_platform/backend/backend/data/notifications_test.pyautogpt_platform/backend/backend/api/features/library/_add_to_library.py
📚 Learning: 2026-05-07T18:48:14.242Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 13040
File: autogpt_platform/backend/backend/blocks/llm.py:0-0
Timestamp: 2026-05-07T18:48:14.242Z
Learning: In this repository, isort may split imports from the same module into separate blocks when some imports are aliased (e.g., `from module import X as Y`) and others are not. Preserve the two-block layout when it results from isort (such as keeping `from openai.types.chat import ChatCompletion as OpenAIChatCompletion` separate from non-aliased imports from `openai.types.chat`). Do not treat that split as a style issue during review; merging them into a single block can fail CI with `Imports are incorrectly sorted and/or formatted`.
Applied to files:
autogpt_platform/backend/backend/api/features/library/_add_to_library_test.pyautogpt_platform/backend/backend/data/notifications_test.pyautogpt_platform/backend/backend/api/features/library/_add_to_library.py
📚 Learning: 2026-05-26T14:24:34.866Z
Learnt from: Abhi1992002
Repo: Significant-Gravitas/AutoGPT PR: 13217
File: autogpt_platform/backend/backend/api/features/search/service.py:137-137
Timestamp: 2026-05-26T14:24:34.866Z
Learning: In the Significant-Gravitas/AutoGPT backend, treat `user_id` (an opaque UUID used only for correlation/tracing) as non-PII. Do not flag direct logging of `user_id` in `logger.warning`/`logger.info` statements as a PII exposure issue, as the established convention is to log `user_id` for tracing while reserving PII for fields like email or display name.
Applied to files:
autogpt_platform/backend/backend/api/features/library/_add_to_library_test.pyautogpt_platform/backend/backend/data/notifications_test.pyautogpt_platform/backend/backend/api/features/library/_add_to_library.py
📚 Learning: 2026-06-11T19:39:10.493Z
Learnt from: ntindle
Repo: Significant-Gravitas/AutoGPT PR: 13337
File: autogpt_platform/backend/backend/copilot/graphiti/reranker.py:0-0
Timestamp: 2026-06-11T19:39:10.493Z
Learning: In the Significant-Gravitas/AutoGPT Python backend, when calling the OpenAI Python client `chat.completions.create`, construct the `messages` payload using the concrete typed-dict variants from `openai.types.chat` (e.g., `ChatCompletionSystemMessageParam`, `ChatCompletionUserMessageParam`, etc.) rather than trying to instantiate `ChatCompletionMessageParam` directly. `ChatCompletionMessageParam` is a `Union` alias and is not constructible, so `ChatCompletionMessageParam(role=..., content=...)` should fail type checking. Build each message element with the appropriate concrete typed dict and then annotate the resulting list as `list[ChatCompletionMessageParam]` (e.g., `messages: list[ChatCompletionMessageParam] = [ChatCompletionSystemMessageParam(...), ...]`).
Applied to files:
autogpt_platform/backend/backend/api/features/library/_add_to_library_test.pyautogpt_platform/backend/backend/data/notifications_test.pyautogpt_platform/backend/backend/api/features/library/_add_to_library.py
📚 Learning: 2026-04-21T04:35:34.710Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12865
File: autogpt_platform/backend/backend/data/credit.py:1584-1584
Timestamp: 2026-04-21T04:35:34.710Z
Learning: When reviewing this codebase, don’t flag snake_case attribute names (e.g., `subscription_tier`, `stripe_customer_id`, `top_up_config`) on the app-layer Pydantic `User` model as “wrong” field names. These are correct for the app-layer model and are expected to be mapped from the Prisma-layer camelCase fields (e.g., `subscriptionTier`, `stripeCustomerId`) inside methods like `User.from_db()`. Only Prisma-returned/raw objects would use camelCase, but functions like `get_user_by_id(user_id: str)` are expected to return the Pydantic app-layer model.
Applied to files:
autogpt_platform/backend/backend/data/notifications_test.py
📚 Learning: 2026-05-07T15:32:39.703Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 13033
File: autogpt_platform/backend/backend/data/generate_data.py:111-117
Timestamp: 2026-05-07T15:32:39.703Z
Learning: When reviewing the Python data-generation layer, do not treat missing `user_id`/user filtering in calls to graph-metadata resolvers as a security issue if the `graph_id` inputs are already guaranteed to be user-scoped by earlier upstream SQL (e.g., `WHERE "userId" = ...`). In particular, `_resolve_agent_name(graph_id)` in `generate_data.py` correctly calls `get_graph_metadata(graph_id=graph_id)` without a `user_id` parameter by design, because name resolution must also work for user-executed shared/marketplace agents that the user may not own.
Applied to files:
autogpt_platform/backend/backend/data/notifications_test.py
🔇 Additional comments (3)
autogpt_platform/backend/backend/api/features/library/_add_to_library.py (1)
127-128: LGTM!Also applies to: 150-154
autogpt_platform/backend/backend/api/features/library/_add_to_library_test.py (1)
198-202: LGTM!autogpt_platform/backend/backend/data/notifications_test.py (1)
6-6: LGTM!
…off the API server (Sentry) add_graph_execution runs in both the API server (direct prisma) and the scheduler/executor (no prisma — DB via RPC client). resolve_default_tenancy used the direct prisma client unconditionally, so in the scheduler process the lookup failed and the best-effort try/except silently swallowed it — leaving scheduled executions (the PRIMARY leak source) untenanted. Dispatch the resolver by prisma.is_connected() like every other DB dep in the function; expose resolve_default_tenancy on the DB manager. + RPC-path test. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Fixed (Sentry · Bug — silent no-op in the scheduler process): great catch, this was the important one. Now the resolver is dispatched by Scoping note: the library and notification create paths already run where prisma is connected (the API server and the DB-manager process respectively — |
…pe in _add_to_library Same-layer (api→api) import, verified no circular dependency — so it follows the top-level-import guideline. The data/executor sites keep the call-time local import (cross-layer cycle avoidance; tracked in SECRT-2510). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Addressed (CodeRabbit · module-scope import): moved The |
|
Declined (Sentry · notifications.py:477 — false positive): this finding has the process topology backwards.
# notifications/notifications.py:595 — NotificationManager._should_batch
await get_database_manager_async_client(
should_retry=False
).create_or_add_to_user_notification_batch(user_id, event_type, event)So the entire function body executes in the DatabaseManager process, where prisma is connected. The pre-existing This is the mirror image of the executor fix in this same PR (commit ec389df). |
… framing The comments described tenant-at-creation (the permanent behavior) as a workaround for the startup migration sweep — a transient mechanism this change retires. Once the sweep is gone those comments would send readers hunting for code that no longer exists. Reframed around why born-tenanting is correct (tenancy at creation), kept the durable dual-process RPC-dispatch note, and trimmed the executor wall. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…es, upsert update leaves tenancy alone - resume path asserts get_user_default_team is never called (CREATE-path-only invariant, so a refactor can't silently re-tenant an existing row) - notification upsert 'update' branch asserts no organizationId/teamId, so re-batching never overwrites an existing batch's tenant - drop boot-sweep framing from test docstrings/comments (timeless wording) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
/review |
There was a problem hiding this comment.
📋 Automated Review — PR #13745
PR #13745 — fix(backend): born-tenanted executions/library/notifications — stop the startup-migration tenancy sweep leak
Author: ntindle | Files: 10
🎯 Verdict: APPROVE
PR Description Quality
✅ Has Why (untenanted-row leak refilled by create paths, forcing a recurring startup sweep of ~4852 rows), What (three create funnels now stamp default org/team), and How (shared best-effort resolve_default_tenancy, create-path-only guard, RPC dispatch for the prisma-less scheduler/executor). One caveat: the description promises an INFO "watch the leak close" log that is not present in the code (see Should Fix).
What This PR Does
New graph executions, library-agent bookmarks, and notification batches were being created without an organizationId/teamId, so they leaked into an untenanted pool that a startup migration had to keep sweeping. This PR stamps each of those three create paths with the acting user's own default org/team at creation time (via a shared, exception-swallowing resolve_default_tenancy), so the rows are "born tenanted" and the sweep becomes a steady-state no-op. Resume/requeue paths are deliberately untouched, and the resolver derives tenancy solely from the authenticated user identity.
Specialist Findings
🛡️ Security ✅ — Traced the trust boundary: resolve_default_tenancy → get_user_default_team(user_id) resolves strictly to the authenticated caller's own personal org/workspace (orgs/db.py:146-172); no attacker-controllable path can stamp another tenant, and explicit org/team is preserved. Change is security-positive. One 🔵 note: the blanket except Exception (orgs/db.py:187) fails open, so a transient DB blip silently produces untenanted rows again.
🏗️ Architecture ✅ — Sound single-resolver design with correct create-vs-resume isolation and proper RPC registration on both DB-manager facades.
🟠 Duplicated connection dispatch in add_graph_execution (executor/utils.py:1309) re-computes prisma.is_connected() separately from the existing db-selection block at 1230-1237 — two "am I connected?" branches in one function.
🟡 backend.data/backend.executor now import "up" into backend.api.features.orgs.db, forcing call-time imports to dodge a cycle; resolver belongs in a lower shared layer (follow-up).
⚡ Performance ✅ — Net win: eliminates a recurring startup sweep of thousands of rows. Cost is a bounded +1 lookup (or +1 RPC round-trip in the scheduler) on the untenanted create branch only; steady-state born-tenanted traffic short-circuits. 🟡 Uncached, so a user firing many executions — or a deep sub-graph fan-out inheriting an untenanted parent — re-resolves per call (executor/utils.py:1300); a per-process user_id→(org,team) cache would collapse this.
🧪 Testing assert_not_called()) and (None,None)/raise-swallow are covered. Two gaps remain: the headline legacy empty-string org (organization_id="" from scheduler.py:1101) is never tested — every fallback test passes None (utils_test.py); and the create-path ExecutionContext tenancy assertion (billing/nested runs) exists on the resume test but not the create/subgraph test.
🟠 Add organization_id="" fallback test + create-path ExecutionContext assertion.
📖 Quality ✅ — Clean, well-documented; comments explain why (tenancy reasoning), tests are scenario-named. Only 🔵 nits: two call-time imports lack an at-site rationale, and the "stamp-when-non-null" idiom repeats across ~3 sites.
📦 Product executor/utils.py), so the team can't observe the leak closing to justify retiring the sweep (SECRT-2476).
📬 Discussion team_id overwrite) are resolved, and the prior high-severity test-gap threads are closed in-tree. Three unrebutted MEDIUM reviewer notes remain (hot-path caching, data→API layering, wasted resolver work on the notification update branch) — accept-and-defer is fine. No human approval yet (REVIEW_REQUIRED).
🔎 QA ✅ — Verified live in a running stack: executions (API and scheduled/RPC path), library agents, and notification batches are all born with organizationId stamped; 0 untenanted executions remained and no scheduler/executor tracebacks. The trickiest branch — a scheduled run resolving tenancy via DB-manager RPC — worked. 🟠 Observed teamId is populated on notification batches but NULL on created library agents; worth confirming whether library rows should also carry teamId.
🟠 Should Fix
- Missing born-tenanted INFO log promised by the description (
executor/utils.pycreate-fallback block) — the "watch the leak close" observability signal the PR relies on to retire the sweep does not exist; only a failure-pathwarninginorgs/db.py:189was added. Add the INFO log or drop the claim. (Flagged by: product, QA — 2 specialists) - Test the headline empty-string-org leak source (
executor/utils_test.py) — legacy schedules passorganization_id=""(scheduler.py:1101), and the fallback relies on the falsy guard treating""as unset; no test passes"", so a regression tois Nonewould silently reopen the exact leak with the suite green. Addtest_add_graph_execution_empty_string_org_triggers_fallback. (Flagged by: testing, architect — 2 specialists) - Assert resolved tenancy flows into the create-path
ExecutionContext(executor/utils_test.py) — the billing/nested-run guarantee is asserted on the resume test but not on create/subgraph; capture theexecution_contextpassed toto_graph_execution_entry. (Flagged by: testing) teamIdNULL on created library agents vs populated on notifications (_add_to_library.py:125) — QA observed the divergence live; confirm whether theTeam: {connect}stamp persists on the library create branch, and add a DB-level assertion. (Flagged by: QA)- Duplicated connection dispatch in
add_graph_execution(executor/utils.py:1309) — fold the resolver into the existing db-selection block so there is a singleis_connected()decision point. (Flagged by: architect)
🟡 Nice to Have
- Cache default-tenancy per user (
executor/utils.py:1300,data/notifications.py:468) — a short-TTL per-processuser_id→(org,team)cache neutralizes the per-create lookup and the sub-graph fan-out N+1. (performance, discussion) - Distinguish transient DB errors from "no org" in the fail-open handler (
orgs/db.py:187) — so a DB blip doesn't silently refill the untenanted pool; at minimum log at error level with a metric. (security) - Relocate the tenancy resolver to a lower shared layer (
data/notifications.py:473) — removes the API↔data dependency inversion and the call-time imports it forces. (architect, discussion) - Align the notifications guard with the executor's falsy check (
data/notifications.py:472) —is Nonethere vsnot organization_idin the executor; latent inconsistency for empty-string org. (architect)
🔵 Nits
- Unexplained call-time imports (
executor/utils.py:1300,data/notifications.py:472) — add a one-line circular-import rationale at each site. (quality) - Stale mock-target comment (
executor/conftest.py:44) — says the code importsget_user_default_team; it actually importsresolve_default_tenancy(which wraps it). (architect) - Change-relative comment tense (
_add_to_library.py:148-151) — reword "risked disconnecting" to state the standing invariant. (architect) - Repeated stamp-when-non-null idiom across ~3 files — a small shared helper would centralize the convention. (quality)
Human Review Needed
NO — This stamps rows with the acting user's own personal org/team; it changes no authentication/authorization logic and opens no cross-tenant path (security traced the resolver end-to-end and confirmed tenancy is derived from the authenticated identity alone). QA verified the behavior live with zero untenanted rows and no crashes.
Risk Assessment
Merge risk: LOW | Rollback: EASY (isolated, additive stamping guarded to the create branch; resume/requeue untouched)
CI Status
- GitHub CI (per discussion specialist): ~42/42 functional checks green (tests 3.11/3.12/3.13, type-check, lint, e2e, CodeQL, Snyk, codecov);
MERGEABLE, no conflicts;Vercel Agent Reviewskipped (backend-only, expected). Live status not independently re-fetched at report time. - Local harness: ✅ All 5 checks pass (frontend lint/types/unit/build, backend
poetry run lint).
UI Testing — Variant Results
✅ local: Born-tenanted org stamping verified across API, scheduler (RPC), library, and notifications with zero untenanted executions and no crashes; only open item is teamId being NULL on created library agents while notifications get both org and team.
- medium: Observed at runtime: a newly created LibraryAgent is born with organizationId set but teamId NULL, whereas a NotificationBatch created for the same user in the same session is born with BOTH organizationId and teamId (2f70dded…). Since resolve_default_tenancy demonstrably returns a valid default team, the Team.connect on the library create branch does not appear to be taking effect (or the create path routes through create_library_agent, which likewise left teamId null).
- low: The PR description states the executor fallback 'logs at INFO (born-tenanted fallback: resolved default org/team for user …)' so the leak can be watched closing, but the deployed code emits no such log statement when the fallback resolves and applies a default org/team.
✅ hosted: Live testing confirms executions, notification batches, and library agents are born tenanted at their create funnels, with (None,None) resolving gracefully and 35 PR unit tests passing.
github-project-automation
Bot
moved this from 🆕 Needs initial review
to 👍🏼 Mergeable
in AutoGPT development kanban
github-project-automation
Bot
moved this from 👍🏼 Mergeable
to ✅ Done
in AutoGPT development kanban
1 participant
Why
New rows for executions, library agents, and notification batches are created untenanted (no
organizationId/teamId) whenever the caller doesn't supply one. Every boot, the startup org-migration sweep has to find and backfill that untenanted pool — on dev that's ~4852 executions swept on every startup. It's a slow, repeated no-op-shaped write, and the pool never stops refilling because the create paths keep leaking new untenanted rows.The leaks trace to three create sites:
create_graph_executiononly stamps org/team when a non-null value is passed (**({"organizationId": org} if org else {})). Its sole funnel,add_graph_execution, doesn't resolve a default. Untenanted rows come from: legacy schedules (GraphExecutionJobArgs.organization_id = ""→ falsy → dropped), sub-graphs inheriting an untenanted parent, and any path that omits tenancy.add_graph_to_librarydoes a directLibraryAgent.prisma().create(...)with no org/team (unlikecreate_library_agent, which already resolves a default team).create_or_add_to_user_notification_batchnever set org/team.What
Resolve the user's default org/team at each create funnel so the rows are born tenanted, using the same
get_user_default_teamself-healing resolver the webhook/copilot/external-API paths already use.resolve_default_tenancy(api/features/orgs/db.py) — new best-effort wrapper overget_user_default_team: any raised lookup yields(None, None)plus a warning log, so tenancy resolution can never abort the operation that needed it. Also exported onDatabaseManager/DatabaseManagerAsyncClientso processes without a direct Prisma connection can resolve over RPC.add_graph_execution(executor/utils.py) — on the create path only, when neitherorganization_idnorteam_idis set (a falsy check, so the legacy""fromGraphExecutionJobArgscounts as unset), resolve the default org/team and pass the result down tocreate_graph_execution. The resolver is dispatched the same way every other DB dep in that function is — direct whenprisma.is_connected(), else viaget_database_manager_async_client()— because the scheduler/executor processes have no Prisma connection and that is exactly where scheduled runs are created. This covers the sub-graph path (AgentExecutorBlockpassesexecution_context.organization_id; when the parent is untenanted that'sNone, so the fallback fires) and legacy schedules. The resolved value also flows into the runtimeExecutionContextbuilt just below, so billing and nested runs are tenant-aware.add_graph_to_library(library/_add_to_library.py) — resolve the default team and stamporganizationId+Team.connecton the create branch only. TheUniqueViolationErrorupdate branch deliberately leaves tenancy untouched: an existing bookmark already carries its tenancy, and re-tagging there risked disconnecting an existing team when no default resolves.create_or_add_to_user_notification_batch(data/notifications.py) — add optionalorganization_id/team_idparams (defaultNone); when both are unset, resolve internally and stamp the create branch of the upsert only.UserNotificationBatchhas noTeamrelation in the schema, soteamIdis stamped as a scalar (unlikeLibraryAgent, which declaresTeamand therefore usesconnect). The single caller is unchanged (resolves internally).create_graph_execution'sif org else {}is intentionally left alone — the value is resolved upstream so what's passed in is already non-null.Guard rails
graph_exec_idbranch and theexecution_contextbackfill at utils.py). Re-resolving there would risk re-tenanting an existing row under a different org — so we don't.resolve_default_tenancyreturns(None, None); we leave the row untenanted and let the boot sweep catch it (unchanged fallback behavior).team_idwithout an org survives untouched.org_credit.py) is deliberately not touched — that's a separate decision.This makes the startup org-migration sweep a fast no-op in steady state (nothing new to backfill), while the sweep stays in place as a safety net — to be retired to a one-off Job later per SECRT-2476.
How (tested)
Scenario-named tests, mocked at the Prisma/RPC boundary per existing patterns:
executor/utils_test.py
test_add_graph_execution_born_tenanted_resolves_default_team— no org → row created with the user's default org/team.test_add_graph_execution_born_tenanted_via_rpc_when_prisma_disconnected— no Prisma connection → resolution goes through the DB-manager RPC client (the scheduler/executor case).test_add_graph_execution_no_default_team_stays_untenanted—(None, None)→ no crash, row stays untenanted.test_add_graph_execution_default_team_lookup_failure_stays_untenanted— resolver raises → run still created, untenanted.test_add_graph_execution_explicit_org_not_overridden— explicit org → fallback does not fire.test_add_graph_execution_explicit_team_id_preserved_when_org_absent— explicit team, no org → fallback does not fire, team survives.test_add_graph_execution_subgraph_untenanted_parent_triggers_fallback— sub-graph with untenanted parent → fallback fires, child born tenanted.test_add_graph_execution_is_repeatablestays green (new autouse fixture defaults the resolver to(None, None)).library/_add_to_library_test.py
organizationId+Team.connectcreate input);(None, None)→ untagged, no crash; resolver raises → untagged, no crash; UniqueViolation update → tenancy left untouched.data/notifications_test.py
orgs/routes_test.py
resolve_default_tenancypasses through the resolved pair and converts a raised lookup into(None, None).Checks:
isort --profile black+blackon touched files;pyrighttouched files → 0 errors, 0 warnings.🤖 Generated with Claude Code
https://claude.ai/code/session_01NBwmh7CkHiF8vKLBf2GWTU
Note
Medium Risk
Touches core execution creation and tenancy stamping across API and scheduler paths; guards limit changes to create paths and best-effort failure handling reduces blast radius.
Overview
Stops new executions, library bookmarks, and notification batches from being created without
organizationId/teamId, which was refilling the pool the startup org-migration sweep has to backfill.Adds
resolve_default_tenancyin orgsdb— a best-effort wrapper aroundget_user_default_teamthat returns(None, None)on failure so creates never abort — and exposes it on the DB manager RPC for scheduler/executor processes without direct Prisma.add_graph_execution(create path only): when neither org nor team is set, resolves defaults and passes them intocreate_graph_execution; uses direct Prisma in the API server andresolve_default_tenancyvia RPC when Prisma is disconnected. Resume/requeue paths are unchanged so existing rows are not re-tagged.add_graph_to_library: stamps org/team on newLibraryAgentcreates only; UniqueViolation restore updates leave tenancy alone.create_or_add_to_user_notification_batch: optionalorganization_id/team_id; if both unset, resolves defaults and stamps the upsert create branch only (not update).Tests and an executor autouse fixture default tenancy resolution to
(None, None)so existing tests stay stable.Reviewed by Cursor Bugbot for commit f75a6bd. Bugbot is set up for automated code reviews on this repo. Configure here.