fix(backend/copilot-bot): session-scope bot file uploads so AutoPilot can read them - #13464
Merged
Merged
Conversation
… can read them
The bot uploaded attachments with a session-less WorkspaceManager to
uploads/<uuid>/<name>, while the web UI (api/features/workspace/routes.py)
uploads them session-scoped to /sessions/<session_id>/<name>. The copilot
executor reads a turn's attachments through a session-scoped manager, so it
never found the bot's file — AutoPilot saw the filename but couldn't read the
contents ('saved but the path doesn't exist').
Upload the file the same way the web endpoint does — a session-scoped
WorkspaceManager + write_file(content, filename). Since the bot only resolves
its session when the turn starts (after the upload), add ensure_chat_session()
so the handler resolves/creates the session up front, threads it into the
upload, and caches it for the turn to reuse.
Follow-up to #13427, which shipped the upload pipeline but stored files
session-less.
github-project-automation
Bot
moved this to 🆕 Needs initial review
in AutoGPT development kanban
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📜 Recent review details⏰ Context from checks skipped due to timeout. (13)
WalkthroughThis PR adds per-turn session resolution for Copilot attachment uploads and threads the resolved session id through chat, the bot backend, and message handling. Workspace uploads now carry ChangesSession-scoped attachment uploads
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant MessageHandler
participant sessions
participant BotBackend
participant PlatformLinkingManager
MessageHandler->>sessions: get_session(platform, target_id)
alt session missing
MessageHandler->>BotBackend: ensure_session(...)
BotBackend->>PlatformLinkingManager: ensure_chat_session(...)
PlatformLinkingManager-->>BotBackend: session_id
BotBackend-->>MessageHandler: session_id
MessageHandler->>sessions: set_session(session_id)
end
MessageHandler->>BotBackend: upload_workspace_files(attachments, session_id)
BotBackend->>PlatformLinkingManager: upload_workspace_file(session_id=session_id)
Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
Contributor
🔍 PR Overlap DetectionThis check compares your PR against all other open PRs targeting the same branch to detect potential merge conflicts early. 🔴 Merge Conflicts DetectedThe following PRs have been tested and will have merge conflicts if merged after this PR. Consider coordinating with the authors.
Summary: 1 conflict(s), 0 medium risk, 0 low risk (out of 1 PRs with file overlap) Auto-generated on push. Ignores: |
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
ntindle
previously approved these changes
Jul 1, 2026
github-project-automation
Bot
moved this from 🆕 Needs initial review
to 👍🏼 Mergeable
in AutoGPT development kanban
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against 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/copilot/bot/handler.py`:
- Around line 115-136: The same-turn attachment session flow in handler.py is
still best-effort, which can let uploads use one session while _stream_batch
later resolves another from Redis. Update the session resolution path in the
attachment handling block and the related streaming path so the locally resolved
session_id is carried through start_chat_turn/_stream_batch instead of being
re-read from sessions.get_session(). If ensure_session(),
sessions.get_session(), or sessions.set_session() fails, surface that as an
attachment/upload failure rather than continuing with a sessionless upload.
In `@autogpt_platform/backend/backend/platform_linking/chat.py`:
- Around line 121-137: The fallback in _resolve_or_create_session() is
incorrectly creating a new ChatSession when a caller-supplied session_id cannot
be found, which breaks attachment-bearing turns by changing the session ID after
files were already stored. Update _resolve_or_create_session() and the
start_chat_turn() flow to preserve the provided session_id for uploaded-file
turns: if get_chat_session() misses an explicit session, fail or retry instead
of calling create_chat_session(). Keep the session resolution behavior only for
non-attachment cases, and ensure any logic around the session_id parameter does
not silently switch IDs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7f9d2316-c1b4-4c85-bebd-63cec0f2e2f3
📒 Files selected for processing (8)
autogpt_platform/backend/backend/copilot/bot/bot_backend.pyautogpt_platform/backend/backend/copilot/bot/handler.pyautogpt_platform/backend/backend/copilot/bot/handler_test.pyautogpt_platform/backend/backend/copilot/bot/sessions.pyautogpt_platform/backend/backend/platform_linking/chat.pyautogpt_platform/backend/backend/platform_linking/chat_test.pyautogpt_platform/backend/backend/platform_linking/manager.pyautogpt_platform/backend/backend/platform_linking/models.py
📜 Review details
⏰ Context from checks skipped due to timeout. (13)
- GitHub Check: check API types
- GitHub Check: Seer Code Review
- GitHub Check: test (3.13)
- GitHub Check: test (3.12)
- GitHub Check: end-to-end tests
- GitHub Check: Analyze (typescript)
- GitHub Check: lint
- GitHub Check: Analyze (python)
- GitHub Check: type-check (3.11)
- GitHub Check: type-check (3.12)
- GitHub Check: type-check (3.13)
- GitHub Check: Check PR Status
- GitHub Check: test (3.11)
🧰 Additional context used
📓 Path-based instructions (3)
autogpt_platform/backend/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
autogpt_platform/backend/**/*.py: Use Python 3.11 (required; managed by Poetry via pyproject.toml) for backend development
Always run 'poetry run format' (Black + isort) before linting in backend development
Always run 'poetry run lint' (ruff) after formatting in backend development
autogpt_platform/backend/**/*.py: Usepoetry run ...command for executing Python package dependencies
Use top-level imports only — avoid local/inner imports except for lazy imports of heavy optional dependencies likeopenpyxl
Use absolute imports withfrom backend.module import ...for cross-package imports; single-dot relative imports are acceptable for sibling modules within the same package; avoid double-dot relative imports
Do not use duck typing — avoidhasattr/getattr/isinstancefor type dispatch; use typed interfaces/unions/protocols instead
Use Pydantic models over dataclass/namedtuple/dict for structured data
Do not use linter suppressors — no# type: ignore,# noqa,# pyright: ignore; fix the type/code instead
Prefer list comprehensions over manual loop-and-append patterns
Use early return with guard clauses first to avoid deep nesting
Use%sfor deferred interpolation indebuglog statements for efficiency; use f-strings elsewhere for readability (e.g.,logger.debug("Processing %s items", count)vslogger.info(f"Processing {count} items"))
Sanitize error paths by usingos.path.basename()in error messages to avoid leaking directory structure
Be aware of TOCTOU (Time-Of-Check-Time-Of-Use) issues — avoid check-then-act patterns for file access and credit charging
Usetransaction=Truefor Redis pipelines to ensure atomicity on multi-step operations
Usemax(0, value)guards for computed values that should never be negative
Keep files under ~300 lines; if a file grows beyond this, split by responsibility (extract helpers, models, or a sub-module into a new file)
Keep functions under ~40 lines; extract named helpers when a function grows longer
...
Files:
autogpt_platform/backend/backend/copilot/bot/sessions.pyautogpt_platform/backend/backend/platform_linking/models.pyautogpt_platform/backend/backend/platform_linking/manager.pyautogpt_platform/backend/backend/copilot/bot/handler.pyautogpt_platform/backend/backend/copilot/bot/handler_test.pyautogpt_platform/backend/backend/copilot/bot/bot_backend.pyautogpt_platform/backend/backend/platform_linking/chat.pyautogpt_platform/backend/backend/platform_linking/chat_test.py
autogpt_platform/{backend,autogpt_libs}/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Format Python code with
poetry run format
Files:
autogpt_platform/backend/backend/copilot/bot/sessions.pyautogpt_platform/backend/backend/platform_linking/models.pyautogpt_platform/backend/backend/platform_linking/manager.pyautogpt_platform/backend/backend/copilot/bot/handler.pyautogpt_platform/backend/backend/copilot/bot/handler_test.pyautogpt_platform/backend/backend/copilot/bot/bot_backend.pyautogpt_platform/backend/backend/platform_linking/chat.pyautogpt_platform/backend/backend/platform_linking/chat_test.py
autogpt_platform/backend/**/*_test.py
📄 CodeRabbit inference engine (autogpt_platform/backend/AGENTS.md)
autogpt_platform/backend/**/*_test.py: Use pytest with snapshot testing for API responses
Colocate test files with source files using*_test.pynaming convention
Mock at boundaries — mock where the symbol is used, not where it's defined; after refactoring, update mock targets to match new module paths
UseAsyncMockfromunittest.mockfor async functions in tests
When writing tests, use Test-Driven Development (TDD): write failing tests marked with@pytest.mark.xfailbefore implementation, then remove the marker once the implementation is complete
When creating snapshots in tests, usepoetry run pytest path/to/test.py --snapshot-update; always review snapshot changes withgit diffbefore committing
Files:
autogpt_platform/backend/backend/copilot/bot/handler_test.pyautogpt_platform/backend/backend/platform_linking/chat_test.py
🧠 Learnings (14)
📚 Learning: 2026-02-26T17:02:22.448Z
Learnt from: Pwuts
Repo: Significant-Gravitas/AutoGPT PR: 12211
File: .pre-commit-config.yaml:160-179
Timestamp: 2026-02-26T17:02:22.448Z
Learning: Keep the pre-commit hook pattern broad for autogpt_platform/backend to ensure OpenAPI schema changes are captured. Do not narrow to backend/api/ alone, since the generated schema depends on Pydantic models across multiple directories (backend/data/, backend/blocks/, backend/copilot/, backend/integrations/, backend/util/). Narrowing could miss schema changes and cause frontend type desynchronization.
Applied to files:
autogpt_platform/backend/backend/copilot/bot/sessions.pyautogpt_platform/backend/backend/platform_linking/models.pyautogpt_platform/backend/backend/platform_linking/manager.pyautogpt_platform/backend/backend/copilot/bot/handler.pyautogpt_platform/backend/backend/copilot/bot/handler_test.pyautogpt_platform/backend/backend/copilot/bot/bot_backend.pyautogpt_platform/backend/backend/platform_linking/chat.pyautogpt_platform/backend/backend/platform_linking/chat_test.py
📚 Learning: 2026-03-04T08:04:35.881Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12273
File: autogpt_platform/backend/backend/copilot/tools/workspace_files.py:216-220
Timestamp: 2026-03-04T08:04:35.881Z
Learning: In the AutoGPT Copilot backend, ensure that SVG images are not treated as vision image types by excluding 'image/svg+xml' from INLINEABLE_MIME_TYPES and MULTIMODAL_TYPES in tool_adapter.py; the Claude API supports PNG, JPEG, GIF, and WebP for vision. SVGs (XML text) should be handled via the text path instead, not the vision path.
Applied to files:
autogpt_platform/backend/backend/copilot/bot/sessions.pyautogpt_platform/backend/backend/copilot/bot/handler.pyautogpt_platform/backend/backend/copilot/bot/handler_test.pyautogpt_platform/backend/backend/copilot/bot/bot_backend.py
📚 Learning: 2026-04-01T04:17:41.600Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12632
File: autogpt_platform/backend/backend/copilot/tools/workspace_files.py:0-0
Timestamp: 2026-04-01T04:17:41.600Z
Learning: When reviewing AutoGPT Copilot tool implementations, accept that `readOnlyHint=True` (provided via `ToolAnnotations`) may be applied unconditionally to *all* tools—even tools that have side effects (e.g., `bash_exec`, `write_workspace_file`, or other write/save operations). Do **not** flag these tools for having `readOnlyHint=True`; this is intentional to enable fully-parallel dispatch by the Anthropic SDK/CLI and has been E2E validated. Only flag `readOnlyHint` issues if they conflict with the established `ToolAnnotations` behavior (e.g., missing/incorrect propagation relative to the intended annotation mechanism).
Applied to files:
autogpt_platform/backend/backend/copilot/bot/sessions.pyautogpt_platform/backend/backend/copilot/bot/handler.pyautogpt_platform/backend/backend/copilot/bot/handler_test.pyautogpt_platform/backend/backend/copilot/bot/bot_backend.py
📚 Learning: 2026-06-06T12:22:37.648Z
Learnt from: anvyle
Repo: Significant-Gravitas/AutoGPT PR: 13302
File: autogpt_platform/backend/backend/copilot/tools/agent_search.py:579-583
Timestamp: 2026-06-06T12:22:37.648Z
Learning: When writing LLM-facing instruction strings that trigger tool calls in this AutoGPT codebase, use the exact registered tool name `view_agent_output` (as defined in `backend/copilot/tools/agent_output.py` via its `name` property and exported via `TOOL_REGISTRY`). Do not reference the bare name `agent_output`, since it is not a valid tool name and will cause tool invocation to fail.
Applied to files:
autogpt_platform/backend/backend/copilot/bot/sessions.pyautogpt_platform/backend/backend/copilot/bot/handler.pyautogpt_platform/backend/backend/copilot/bot/handler_test.pyautogpt_platform/backend/backend/copilot/bot/bot_backend.py
📚 Learning: 2026-03-05T15:42:08.207Z
Learnt from: ntindle
Repo: Significant-Gravitas/AutoGPT PR: 12297
File: .claude/skills/backend-check/SKILL.md:14-16
Timestamp: 2026-03-05T15:42:08.207Z
Learning: In Python files under autogpt_platform/backend (recursively), rely on poetry run format to perform formatting (Black + isort) and linting (ruff). Do not run poetry run lint as a separate step after poetry run format, since format already includes linting checks.
Applied to files:
autogpt_platform/backend/backend/copilot/bot/sessions.pyautogpt_platform/backend/backend/platform_linking/models.pyautogpt_platform/backend/backend/platform_linking/manager.pyautogpt_platform/backend/backend/copilot/bot/handler.pyautogpt_platform/backend/backend/copilot/bot/handler_test.pyautogpt_platform/backend/backend/copilot/bot/bot_backend.pyautogpt_platform/backend/backend/platform_linking/chat.pyautogpt_platform/backend/backend/platform_linking/chat_test.py
📚 Learning: 2026-03-16T16:35:40.236Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12440
File: autogpt_platform/backend/backend/api/features/workflow_import.py:54-63
Timestamp: 2026-03-16T16:35:40.236Z
Learning: Avoid using the word 'competitor' in public-facing identifiers and text. Use neutral naming for API paths, model names, function names, and UI text. Examples: rename 'CompetitorFormat' to 'SourcePlatform', 'convert_competitor_workflow' to 'convert_workflow', '/competitor-workflow' to '/workflow'. Apply this guideline to files under autogpt_platform/backend and autogpt_platform/frontend.
Applied to files:
autogpt_platform/backend/backend/copilot/bot/sessions.pyautogpt_platform/backend/backend/platform_linking/models.pyautogpt_platform/backend/backend/platform_linking/manager.pyautogpt_platform/backend/backend/copilot/bot/handler.pyautogpt_platform/backend/backend/copilot/bot/handler_test.pyautogpt_platform/backend/backend/copilot/bot/bot_backend.pyautogpt_platform/backend/backend/platform_linking/chat.pyautogpt_platform/backend/backend/platform_linking/chat_test.py
📚 Learning: 2026-03-31T15:37:38.626Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12623
File: autogpt_platform/backend/backend/copilot/tools/agent_generator/fixer.py:37-47
Timestamp: 2026-03-31T15:37:38.626Z
Learning: When validating/constructing Anthropic API model IDs in Significant-Gravitas/AutoGPT, allow the hyphen-separated Claude Opus 4.6 model ID `claude-opus-4-6` (it corresponds to `LlmModel.CLAUDE_4_6_OPUS` in `autogpt_platform/backend/backend/blocks/llm.py`). Do NOT require the dot-separated form in Anthropic contexts. Only OpenRouter routing variants should use the dot separator (e.g., `anthropic/claude-opus-4.6`); `claude-opus-4-6` should be treated as correct when passed to Anthropic, and flagged only if it’s used in the OpenRouter path where the dot form is expected.
Applied to files:
autogpt_platform/backend/backend/copilot/bot/sessions.pyautogpt_platform/backend/backend/platform_linking/models.pyautogpt_platform/backend/backend/platform_linking/manager.pyautogpt_platform/backend/backend/copilot/bot/handler.pyautogpt_platform/backend/backend/copilot/bot/handler_test.pyautogpt_platform/backend/backend/copilot/bot/bot_backend.pyautogpt_platform/backend/backend/platform_linking/chat.pyautogpt_platform/backend/backend/platform_linking/chat_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/copilot/bot/sessions.pyautogpt_platform/backend/backend/platform_linking/models.pyautogpt_platform/backend/backend/platform_linking/manager.pyautogpt_platform/backend/backend/copilot/bot/handler.pyautogpt_platform/backend/backend/copilot/bot/handler_test.pyautogpt_platform/backend/backend/copilot/bot/bot_backend.pyautogpt_platform/backend/backend/platform_linking/chat.pyautogpt_platform/backend/backend/platform_linking/chat_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/copilot/bot/sessions.pyautogpt_platform/backend/backend/platform_linking/models.pyautogpt_platform/backend/backend/platform_linking/manager.pyautogpt_platform/backend/backend/copilot/bot/handler.pyautogpt_platform/backend/backend/copilot/bot/handler_test.pyautogpt_platform/backend/backend/copilot/bot/bot_backend.pyautogpt_platform/backend/backend/platform_linking/chat.pyautogpt_platform/backend/backend/platform_linking/chat_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/copilot/bot/sessions.pyautogpt_platform/backend/backend/platform_linking/models.pyautogpt_platform/backend/backend/platform_linking/manager.pyautogpt_platform/backend/backend/copilot/bot/handler.pyautogpt_platform/backend/backend/copilot/bot/handler_test.pyautogpt_platform/backend/backend/copilot/bot/bot_backend.pyautogpt_platform/backend/backend/platform_linking/chat.pyautogpt_platform/backend/backend/platform_linking/chat_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/copilot/bot/sessions.pyautogpt_platform/backend/backend/platform_linking/models.pyautogpt_platform/backend/backend/platform_linking/manager.pyautogpt_platform/backend/backend/copilot/bot/handler.pyautogpt_platform/backend/backend/copilot/bot/handler_test.pyautogpt_platform/backend/backend/copilot/bot/bot_backend.pyautogpt_platform/backend/backend/platform_linking/chat.pyautogpt_platform/backend/backend/platform_linking/chat_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/copilot/bot/sessions.pyautogpt_platform/backend/backend/platform_linking/models.pyautogpt_platform/backend/backend/platform_linking/manager.pyautogpt_platform/backend/backend/copilot/bot/handler.pyautogpt_platform/backend/backend/copilot/bot/handler_test.pyautogpt_platform/backend/backend/copilot/bot/bot_backend.pyautogpt_platform/backend/backend/platform_linking/chat.pyautogpt_platform/backend/backend/platform_linking/chat_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/copilot/bot/sessions.pyautogpt_platform/backend/backend/platform_linking/models.pyautogpt_platform/backend/backend/platform_linking/manager.pyautogpt_platform/backend/backend/copilot/bot/handler.pyautogpt_platform/backend/backend/copilot/bot/handler_test.pyautogpt_platform/backend/backend/copilot/bot/bot_backend.pyautogpt_platform/backend/backend/platform_linking/chat.pyautogpt_platform/backend/backend/platform_linking/chat_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/copilot/bot/sessions.pyautogpt_platform/backend/backend/platform_linking/models.pyautogpt_platform/backend/backend/platform_linking/manager.pyautogpt_platform/backend/backend/copilot/bot/handler.pyautogpt_platform/backend/backend/copilot/bot/handler_test.pyautogpt_platform/backend/backend/copilot/bot/bot_backend.pyautogpt_platform/backend/backend/platform_linking/chat.pyautogpt_platform/backend/backend/platform_linking/chat_test.py
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## dev #13464 +/- ##
==========================================
+ Coverage 74.89% 74.90% +0.01%
==========================================
Files 2588 2588
Lines 194053 194177 +124
Branches 19076 19083 +7
==========================================
+ Hits 145330 145445 +115
- Misses 44571 44577 +6
- Partials 4152 4155 +3
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
…on session resolve Address review on #13464: - Same-named files in a session no longer collide into a misleading 'too large / quota' rejection: write_file(overwrite=True) replaces the prior file, so the ValueError branch again means only size/quota. - If session resolution fails before upload, report the attachments as failed instead of uploading them session-less where AutoPilot can't read them.
…ose the race Address review finding 2b on #13464: concurrent attachment messages on a fresh target could each create their own session and split the files. Resolve the session under a per-target asyncio lock so they converge on one, and carry the resolved session_id straight through _enqueue_and_process -> _stream_batch to the turn instead of re-reading it from Redis — the turn now uses the exact session the files were uploaded to.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
autogpt_platform/backend/backend/copilot/bot/handler.py (1)
130-167: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract session resolution into a helper.
The added lock/try-except block pushes
handle()well past the ~40-line guideline and adds another level of nesting. Extracting it into a small helper (e.g._resolve_session_for_attachments(ctx, target_id) -> str | None) would keephandle()readable and make the resolution logic independently testable.♻️ Suggested extraction
+ async def _resolve_session_for_attachments( + self, ctx: MessageContext, target_id: str + ) -> str | None: + async with self._session_lock(target_id): + try: + session_id = await self._api.ensure_session( + platform=ctx.platform, + platform_user_id=ctx.user_id, + platform_server_id=ctx.server_id, + session_id=await sessions.get_session(ctx.platform, target_id), + ) + await sessions.set_session(ctx.platform, target_id, session_id) + return session_id + except Exception: + logger.exception( + "Failed to resolve session for uploads (user %s)", ctx.user_id + ) + return None + async def handle(self, ctx: MessageContext, adapter: PlatformAdapter) -> None: ... - session_id: str | None = None - file_ids: list[str] - upload_problems: list[tuple[str, str]] if ctx.attachments: - async with self._session_lock(target_id): - try: - session_id = await self._api.ensure_session(...) - await sessions.set_session(ctx.platform, target_id, session_id) - except Exception: - logger.exception(...) - session_id = None + session_id = await self._resolve_session_for_attachments(ctx, target_id) if session_id is None: file_ids = [] upload_problems = [...] else: file_ids, upload_problems = await self._upload_attachments(ctx, session_id) else: + session_id = None file_ids, upload_problems = await self._upload_attachments(ctx)As per coding guidelines, "Keep functions under ~40 lines; extract named helpers when a function grows longer" and "Use early return with guard clauses first to avoid deep nesting."
🤖 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/copilot/bot/handler.py` around lines 130 - 167, The attachment-session resolution logic is nested directly inside handler() and makes the method too long and harder to test. Extract the lock/try-except session lookup into a small helper such as _resolve_session_for_attachments(ctx, target_id) that returns str | None, then call it from handler() and keep the upload flow using early returns/guard clauses to reduce nesting.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/copilot/bot/handler.py`:
- Around line 130-167: The attachment-session resolution logic is nested
directly inside handler() and makes the method too long and harder to test.
Extract the lock/try-except session lookup into a small helper such as
_resolve_session_for_attachments(ctx, target_id) that returns str | None, then
call it from handler() and keep the upload flow using early returns/guard
clauses to reduce nesting.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 777ff244-c102-4008-b489-ad49c38f7174
📒 Files selected for processing (2)
autogpt_platform/backend/backend/copilot/bot/handler.pyautogpt_platform/backend/backend/copilot/bot/handler_test.py
🚧 Files skipped from review as they are similar to previous changes (1)
- autogpt_platform/backend/backend/copilot/bot/handler_test.py
📜 Review details
⏰ Context from checks skipped due to timeout. (13)
- GitHub Check: check API types
- GitHub Check: end-to-end tests
- GitHub Check: Seer Code Review
- GitHub Check: Check PR Status
- GitHub Check: type-check (3.11)
- GitHub Check: test (3.11)
- GitHub Check: type-check (3.13)
- GitHub Check: lint
- GitHub Check: test (3.13)
- GitHub Check: type-check (3.12)
- GitHub Check: test (3.12)
- GitHub Check: Analyze (python)
- GitHub Check: Analyze (typescript)
🧰 Additional context used
📓 Path-based instructions (2)
autogpt_platform/backend/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
autogpt_platform/backend/**/*.py: Use Python 3.11 (required; managed by Poetry via pyproject.toml) for backend development
Always run 'poetry run format' (Black + isort) before linting in backend development
Always run 'poetry run lint' (ruff) after formatting in backend development
autogpt_platform/backend/**/*.py: Usepoetry run ...command for executing Python package dependencies
Use top-level imports only — avoid local/inner imports except for lazy imports of heavy optional dependencies likeopenpyxl
Use absolute imports withfrom backend.module import ...for cross-package imports; single-dot relative imports are acceptable for sibling modules within the same package; avoid double-dot relative imports
Do not use duck typing — avoidhasattr/getattr/isinstancefor type dispatch; use typed interfaces/unions/protocols instead
Use Pydantic models over dataclass/namedtuple/dict for structured data
Do not use linter suppressors — no# type: ignore,# noqa,# pyright: ignore; fix the type/code instead
Prefer list comprehensions over manual loop-and-append patterns
Use early return with guard clauses first to avoid deep nesting
Use%sfor deferred interpolation indebuglog statements for efficiency; use f-strings elsewhere for readability (e.g.,logger.debug("Processing %s items", count)vslogger.info(f"Processing {count} items"))
Sanitize error paths by usingos.path.basename()in error messages to avoid leaking directory structure
Be aware of TOCTOU (Time-Of-Check-Time-Of-Use) issues — avoid check-then-act patterns for file access and credit charging
Usetransaction=Truefor Redis pipelines to ensure atomicity on multi-step operations
Usemax(0, value)guards for computed values that should never be negative
Keep files under ~300 lines; if a file grows beyond this, split by responsibility (extract helpers, models, or a sub-module into a new file)
Keep functions under ~40 lines; extract named helpers when a function grows longer
...
Files:
autogpt_platform/backend/backend/copilot/bot/handler.py
autogpt_platform/{backend,autogpt_libs}/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Format Python code with
poetry run format
Files:
autogpt_platform/backend/backend/copilot/bot/handler.py
🧠 Learnings (14)
📚 Learning: 2026-02-26T17:02:22.448Z
Learnt from: Pwuts
Repo: Significant-Gravitas/AutoGPT PR: 12211
File: .pre-commit-config.yaml:160-179
Timestamp: 2026-02-26T17:02:22.448Z
Learning: Keep the pre-commit hook pattern broad for autogpt_platform/backend to ensure OpenAPI schema changes are captured. Do not narrow to backend/api/ alone, since the generated schema depends on Pydantic models across multiple directories (backend/data/, backend/blocks/, backend/copilot/, backend/integrations/, backend/util/). Narrowing could miss schema changes and cause frontend type desynchronization.
Applied to files:
autogpt_platform/backend/backend/copilot/bot/handler.py
📚 Learning: 2026-03-04T08:04:35.881Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12273
File: autogpt_platform/backend/backend/copilot/tools/workspace_files.py:216-220
Timestamp: 2026-03-04T08:04:35.881Z
Learning: In the AutoGPT Copilot backend, ensure that SVG images are not treated as vision image types by excluding 'image/svg+xml' from INLINEABLE_MIME_TYPES and MULTIMODAL_TYPES in tool_adapter.py; the Claude API supports PNG, JPEG, GIF, and WebP for vision. SVGs (XML text) should be handled via the text path instead, not the vision path.
Applied to files:
autogpt_platform/backend/backend/copilot/bot/handler.py
📚 Learning: 2026-04-01T04:17:41.600Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12632
File: autogpt_platform/backend/backend/copilot/tools/workspace_files.py:0-0
Timestamp: 2026-04-01T04:17:41.600Z
Learning: When reviewing AutoGPT Copilot tool implementations, accept that `readOnlyHint=True` (provided via `ToolAnnotations`) may be applied unconditionally to *all* tools—even tools that have side effects (e.g., `bash_exec`, `write_workspace_file`, or other write/save operations). Do **not** flag these tools for having `readOnlyHint=True`; this is intentional to enable fully-parallel dispatch by the Anthropic SDK/CLI and has been E2E validated. Only flag `readOnlyHint` issues if they conflict with the established `ToolAnnotations` behavior (e.g., missing/incorrect propagation relative to the intended annotation mechanism).
Applied to files:
autogpt_platform/backend/backend/copilot/bot/handler.py
📚 Learning: 2026-06-06T12:22:37.648Z
Learnt from: anvyle
Repo: Significant-Gravitas/AutoGPT PR: 13302
File: autogpt_platform/backend/backend/copilot/tools/agent_search.py:579-583
Timestamp: 2026-06-06T12:22:37.648Z
Learning: When writing LLM-facing instruction strings that trigger tool calls in this AutoGPT codebase, use the exact registered tool name `view_agent_output` (as defined in `backend/copilot/tools/agent_output.py` via its `name` property and exported via `TOOL_REGISTRY`). Do not reference the bare name `agent_output`, since it is not a valid tool name and will cause tool invocation to fail.
Applied to files:
autogpt_platform/backend/backend/copilot/bot/handler.py
📚 Learning: 2026-03-05T15:42:08.207Z
Learnt from: ntindle
Repo: Significant-Gravitas/AutoGPT PR: 12297
File: .claude/skills/backend-check/SKILL.md:14-16
Timestamp: 2026-03-05T15:42:08.207Z
Learning: In Python files under autogpt_platform/backend (recursively), rely on poetry run format to perform formatting (Black + isort) and linting (ruff). Do not run poetry run lint as a separate step after poetry run format, since format already includes linting checks.
Applied to files:
autogpt_platform/backend/backend/copilot/bot/handler.py
📚 Learning: 2026-03-16T16:35:40.236Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12440
File: autogpt_platform/backend/backend/api/features/workflow_import.py:54-63
Timestamp: 2026-03-16T16:35:40.236Z
Learning: Avoid using the word 'competitor' in public-facing identifiers and text. Use neutral naming for API paths, model names, function names, and UI text. Examples: rename 'CompetitorFormat' to 'SourcePlatform', 'convert_competitor_workflow' to 'convert_workflow', '/competitor-workflow' to '/workflow'. Apply this guideline to files under autogpt_platform/backend and autogpt_platform/frontend.
Applied to files:
autogpt_platform/backend/backend/copilot/bot/handler.py
📚 Learning: 2026-03-31T15:37:38.626Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12623
File: autogpt_platform/backend/backend/copilot/tools/agent_generator/fixer.py:37-47
Timestamp: 2026-03-31T15:37:38.626Z
Learning: When validating/constructing Anthropic API model IDs in Significant-Gravitas/AutoGPT, allow the hyphen-separated Claude Opus 4.6 model ID `claude-opus-4-6` (it corresponds to `LlmModel.CLAUDE_4_6_OPUS` in `autogpt_platform/backend/backend/blocks/llm.py`). Do NOT require the dot-separated form in Anthropic contexts. Only OpenRouter routing variants should use the dot separator (e.g., `anthropic/claude-opus-4.6`); `claude-opus-4-6` should be treated as correct when passed to Anthropic, and flagged only if it’s used in the OpenRouter path where the dot form is expected.
Applied to files:
autogpt_platform/backend/backend/copilot/bot/handler.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/copilot/bot/handler.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/copilot/bot/handler.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/copilot/bot/handler.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/copilot/bot/handler.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/copilot/bot/handler.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/copilot/bot/handler.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/copilot/bot/handler.py
🔇 Additional comments (2)
autogpt_platform/backend/backend/copilot/bot/handler.py (2)
72-89: 🩺 Stability & AvailabilityConfirm the bot runs as a single process per target.
_session_locksis an in-processasyncio.Lockdict. It correctly serializes concurrent attachment messages within one process, but if the bot is deployed with multiple replicas/workers that can both receive events for the sametarget_id(e.g., horizontally scaled gateway consumers, not sharded by channel), two processes could still race throughget_session→ensure_session→set_sessionconcurrently and create divergent sessions — the exact failure mode this lock is meant to prevent.Please confirm whether the bot process is guaranteed to be the sole handler for a given target (e.g., single gateway connection per shard, with sharding keyed consistently by guild/channel).
Also applies to: 138-152
193-195: LGTM!Also applies to: 305-357, 478-501
…tart + extract helper Address remaining review on #13464: - Finding 3: for an attachment turn (file_ids + supplied session_id), start_chat_turn now requires that session to still exist instead of silently recreating a different one — recreating would run the turn without the already-uploaded files. Normal (text-only) turns keep the self-healing create-on-miss. - Extract the per-target session resolution out of handle() into _resolve_session_for_attachments to keep handle() under the length guideline.
ntindle
approved these changes
Jul 2, 2026
github-project-automation
Bot
moved this from 👍🏼 Mergeable
to ✅ Done
in AutoGPT development kanban
Aastha204
pushed a commit
to Aastha204/AutoGPT
that referenced
this pull request
Jul 3, 2026
…e, not a host path (Significant-Gravitas#13470) ## Why Discord-bot file attachments upload correctly — AV-scanned (ClamAV via `WorkspaceManager.write_file`), stored session-scoped at `/sessions/<session_id>/<name>` ([Significant-Gravitas#13427](Significant-Gravitas#13427) + [Significant-Gravitas#13464](Significant-Gravitas#13464) + infra GCS perms) — but AutoPilot **still can't read them**. From the model's own trace in dev: > "The system says it was saved to `/tmp/copilot-804ca4e7-…/test.txt` but it doesn't exist." Root cause (`sdk/service.py::_prepare_file_attachments`): non-image attachments are written to the **executor host's** `sdk_cwd` and the model is told *"saved to `<host path>`. Use the Read tool."* Two problems: 1. **In E2B mode (the prod/dev default — `use_e2b_sandbox=True`)** the model's file tools operate on the **remote E2B sandbox filesystem** (`e2b_file_tools.py` deliberately routes non-tool-result paths to `sandbox.files.read`). The host-side `sdk_cwd` file doesn't exist there. 2. The built-in `Read` tool the hint names is **disallowed** in every mode (`SDK_DISALLOWED_TOOLS`). So the model is pointed at a path it cannot see, with a tool it cannot use. Web-uploaded files work because the model reads them via `read_workspace_file` (observed in production: `Read test.txt from workspace:/sessions/<id>/test.txt`) — which resolves by `file_id` from the workspace, independent of any local disk. ## What Point the model at `read_workspace_file` with the `file_id` for non-image attachments, instead of a host path + the disallowed `Read` tool. ## How In `_prepare_file_attachments`: - Non-image file descriptions now carry `file_id: <id>` instead of the host path. - The hint instructs `read_workspace_file` (with `save_to_path` mentioned for copying into the working directory) instead of "Use the Read tool". - The `sdk_cwd` copy is **still written**, preserving the working-directory affordance for non-E2B tooling — the model just isn't pointed at it. Blast radius: this function only runs for turns with attached `file_ids`. Web attachment turns get the same instruction that already matches their observed working behaviour; everything else is untouched. Completes the bot file-upload chain: Significant-Gravitas#13427 (upload pipeline) → infra#348 (GCS write perms) → Significant-Gravitas#13464 (session-scoped storage) → **this** (readable reference). ## Testing - TDD: updated/added tests in `sdk/service_test.py::TestPrepareFileAttachments` pinning the new hint (file_id present, host path absent, `read_workspace_file` named, no "Use the Read tool") — failed before the fix, 7/7 pass after. - No-regression check: full `service_test.py` run compared against clean `dev` — identical pre-existing failure set (env-dependent tests under `--noconftest`), zero new failures. - `ruff` / `black` / `isort` clean.
gaurav0107
pushed a commit
to gaurav0107/AutoGPT
that referenced
this pull request
Jul 7, 2026
… on bot turns (Significant-Gravitas#13477) ## Why The bot enqueues copilot turns directly into the executor (`enqueue_copilot_turn`), bypassing the two gates that meter the web UI: the route-level `enforce_payment_paywall` dependency and the dispatcher's `check_rate_limit`. A `NO_TIER` user (paywall on) or a user over their daily/weekly USD cap gets **unmetered free AutoPilot usage via Discord**. ## What Enforce the same subscription paywall + usage rate limits at the bot's turn entry, reusing the platform's own gate functions — no parallel implementation that can drift. The gate also runs before attachment uploads, so a denied user's files are never scanned or stored. ## How - **`evaluate_turn_gate(user_id)`** (`platform_linking/chat.py`, split into `_check_paywall` + `_check_usage_limits`) calls the same functions the web uses: - `is_user_paywalled` → `paywalled` denial with a **Subscribe** button (links to `/settings/billing`) - `get_global_rate_limits` + `check_rate_limit` (same LaunchDarkly per-tier multipliers + config fallbacks) → `rate_limited` denial with the window + reset countdown from `RateLimitExceeded` (e.g. *"You've reached your daily usage limit. Resets in 5h 30m."* — recomputed live at every denial) and an **Upgrade for higher limits** button - **Fail-closed on any lookup failure** (tier lookup, LD, Redis — expected or unexpected exceptions alike): returns an `unavailable` denial ("temporarily unavailable — try again") instead of letting an unmetered turn through, mirroring the web route's 503 behaviour. - **Gate placement — two checkpoints:** - `start_chat_turn` (authoritative): a denied turn returns a `TurnDenial` on `ChatTurnHandle` (RPC-safe) instead of enqueuing — nothing persisted, no stream created, no LLM run. - `ensure_chat_session` (pre-upload): attachment-bearing messages resolve their session before uploading (Significant-Gravitas#13464's flow), so the gate runs there too via `EnsureSessionResult{session_id, denial}` — the bot renders the denial and **skips the upload entirely** (no AV scan, no storage write for denied users). The double evaluation on allowed attachment turns is a couple of cheap reads. - The handler renders denials through one shared `_send_denial` (message + CTA button, `turn_denied` analytics event); buttons degrade to plain text when `FRONTEND_BASE_URL` isn't configured (Discord rejects relative button URLs). - `NO_TIER` with `ENABLE_PLATFORM_PAYMENT` **off** falls back to BASIC limits (same as web), so local/beta testers keep access. ## Testing Live-tested on Discord (local hybrid stack, this branch): - **Allowed turn**: gate passes silently, turn runs normally. - **Capped, text**: bot replies *"You've reached your daily usage limit. Resets in 10h 59m."* with the **Upgrade for higher limits** button → billing page; linker logs `Bot chat turn denied (rate_limited)`, nothing enqueued. - **Capped, with attachment**: single clean denial. (Initially the file uploaded before the denial; the `ensure_chat_session` gate added in review now blocks the upload too — covered by unit tests below.) - Denial forced via `CHAT_DAILY_COST_LIMIT_MICRODOLLARS=0` (cap 0 = "no spend allowed" → deterministic `RateLimitExceeded`). Unit tests: gate branches (paywalled / rate-limited / unavailable / fail-closed on unexpected errors / within-limits), denial rendering (button vs plain message), `stream_chat` raising on denial, denied-user-never-uploads (handler + `ensure_chat_session`) — 113 tests green across `platform_linking` + `copilot/bot`. `ruff`/`black`/`isort` clean.
Issues attributed to commits in this pull requestThis pull request was merged and Sentry observed the following issues:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
The Discord CoPilot bot's file-upload feature (#13427) uploaded attachments with a session-less
WorkspaceManager, storing them atuploads/<uuid>/<name>. The web UI (api/features/workspace/routes.py) uploads the same files session-scoped to/sessions/<session_id>/<name>.The copilot executor reads a turn's attachments through a session-scoped manager, so it never found the bot's file. Symptom in dev: AutoPilot saw the filename but couldn't read the contents ("saved but the path doesn't exist") — while an identical file uploaded via the web UI read fine (
Read test.txt from workspace:/sessions/<id>/test.txt).Follow-up to #13427, which shipped the upload pipeline but stored files session-less.
What
Store bot-uploaded attachments the same way the web endpoint does — session-scoped — so AutoPilot reads them identically.
How
upload_workspace_filenow usesWorkspaceManager(owner, workspace, session_id)+write_file(content, filename, …)— the exact documented pattern the web upload endpoint uses (workspace/routes.py). Dropped the ad-hocuploads/<uuid>/path (and theos.pathusage — workspace paths are POSIX).ensure_chat_sessionRPC lets the handler resolve/create the session before uploading, thread it into the upload, and cache it so the turn reuses the same session.start_chat_turnshares the same_resolve_or_create_sessionhelper.session_idonWorkspaceUploadRequest;ensure_chat_sessionexposed on the manager + client;bot_backend.ensure_session;sessions.get_session.No new storage/DB API — only the documented
WorkspaceManager(session_id)+write_file.Testing
poetry run pytest backend/platform_linking/chat_test.py backend/copilot/bot/handler_test.py backend/copilot/bot/bot_backend_test.py— green (incl. new tests: session-scoped upload,ensure_chat_sessionreuse/create/unlinked, handler ensure-session flow).ruff/black/isortclean.