refactor(backend/copilot): unified queue-backed copilot turns + async sub-AutoPilot + guide-read gate - #12841
Merged
Merged
Conversation
github-project-automation
Bot
moved this to 🆕 Needs initial review
in AutoGPT development kanban
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdjusts stream idle timeout to 30 minutes, adds per-tool soft timeouts that background long-running tool tasks, introduces a per-execution background task registry, implements Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant ToolAdapter as Tool Adapter
participant AsyncTask as Asyncio Task
participant BackgroundRegistry as Background Registry
participant ToolExec as Tool.execute()
participant CheckTool as check_background_tool
Client->>ToolAdapter: execute_tool(tool, args)
activate ToolAdapter
ToolAdapter->>AsyncTask: create Task(tool.execute(args))
activate AsyncTask
alt tool.timeout_seconds is not None
ToolAdapter->>AsyncTask: wait up to timeout (no cancel on expiry)
alt Task completes within timeout
AsyncTask-->>ToolAdapter: result
ToolAdapter-->>Client: MCP success
else timeout elapses
ToolAdapter->>BackgroundRegistry: register_background_task(Task, tool_name)
BackgroundRegistry-->>ToolAdapter: background_id
ToolAdapter-->>Client: MCP {type:"background", background_id}
Note right of AsyncTask: Task continues running
end
else timeout_seconds is None
ToolAdapter->>AsyncTask: await Task until complete
AsyncTask-->>ToolAdapter: result
ToolAdapter-->>Client: MCP success
end
deactivate AsyncTask
deactivate ToolAdapter
Client->>CheckTool: check_background_tool(background_id, wait_seconds, cancel?)
activate CheckTool
CheckTool->>BackgroundRegistry: get_background_task(background_id)
alt missing
CheckTool-->>Client: error (unknown id)
else present
alt cancel == true
CheckTool->>AsyncTask: cancel Task
CheckTool->>BackgroundRegistry: unregister_background_task(background_id)
CheckTool-->>Client: status "cancelled"
else wait_seconds == 0 and not finished
CheckTool-->>Client: status "still_running"
else wait up to wait_seconds
alt finished
CheckTool->>BackgroundRegistry: unregister_background_task(background_id)
CheckTool-->>Client: status "completed" or "error"
else
CheckTool-->>Client: status "still_running" (waited_seconds)
end
end
end
deactivate CheckTool
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
autogpt_platform/backend/backend/copilot/sdk/service_test.py (1)
725-731: Add a behavior test for the timeout exemption path.These assertions lock the constant, but they would still pass if
_run_stream_attemptlater stopped honoring it. A small test for “unresolvedbash_exec/run_agentskipsidle_timeout, while an unresolved non-exempt tool still emits it” would better protect this PR’s main behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@autogpt_platform/backend/backend/copilot/sdk/service_test.py` around lines 725 - 731, Add a test that exercises the timeout-exemption path in _run_stream_attempt: call _run_stream_attempt (or the public wrapper used in tests) with an unresolved tool name listed in _LONG_RUNNING_TOOLS (e.g., "bash_exec" or "run_agent") and assert that no "idle_timeout" event/message is emitted, then call it with an unresolved non-exempt tool name and assert that an "idle_timeout" event/message is emitted; place this new test alongside TestLongRunningTools and reference _LONG_RUNNING_TOOLS, _run_stream_attempt, "bash_exec"/"run_agent", and "idle_timeout" so the test fails if the exemption behavior is removed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@autogpt_platform/backend/backend/copilot/sdk/service_test.py`:
- Around line 725-731: Add a test that exercises the timeout-exemption path in
_run_stream_attempt: call _run_stream_attempt (or the public wrapper used in
tests) with an unresolved tool name listed in _LONG_RUNNING_TOOLS (e.g.,
"bash_exec" or "run_agent") and assert that no "idle_timeout" event/message is
emitted, then call it with an unresolved non-exempt tool name and assert that an
"idle_timeout" event/message is emitted; place this new test alongside
TestLongRunningTools and reference _LONG_RUNNING_TOOLS, _run_stream_attempt,
"bash_exec"/"run_agent", and "idle_timeout" so the test fails if the exemption
behavior is removed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d29615cc-f3f9-4435-862f-cb1049878300
📒 Files selected for processing (2)
autogpt_platform/backend/backend/copilot/sdk/service.pyautogpt_platform/backend/backend/copilot/sdk/service_test.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: check API types
- GitHub Check: Seer Code Review
- GitHub Check: end-to-end tests
- GitHub Check: test (3.11)
- GitHub Check: type-check (3.12)
- GitHub Check: test (3.13)
- GitHub Check: type-check (3.13)
- GitHub Check: test (3.12)
- GitHub Check: type-check (3.11)
- GitHub Check: Check PR Status
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (3)
autogpt_platform/backend/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
autogpt_platform/backend/**/*.py: Use Python 3.11 (required; managed by Poetry via pyproject.toml) for backend development
Always run 'poetry run format' (Black + isort) before linting in backend development
Always run 'poetry run lint' (ruff) after formatting in backend development
autogpt_platform/backend/**/*.py: Usepoetry run ...command for executing Python package dependencies
Use top-level imports only — avoid local/inner imports except for lazy imports of heavy optional dependencies likeopenpyxl
Use absolute imports withfrom backend.module import ...for cross-package imports; single-dot relative imports are acceptable for sibling modules within the same package; avoid double-dot relative imports
Do not use duck typing — avoidhasattr/getattr/isinstancefor type dispatch; use typed interfaces/unions/protocols instead
Use Pydantic models over dataclass/namedtuple/dict for structured data
Do not use linter suppressors — no# type: ignore,# noqa,# pyright: ignore; fix the type/code instead
Prefer list comprehensions over manual loop-and-append patterns
Use early return with guard clauses first to avoid deep nesting
Use%sfor deferred interpolation indebuglog statements for efficiency; use f-strings elsewhere for readability (e.g.,logger.debug("Processing %s items", count)vslogger.info(f"Processing {count} items"))
Sanitize error paths by usingos.path.basename()in error messages to avoid leaking directory structure
Be aware of TOCTOU (Time-Of-Check-Time-Of-Use) issues — avoid check-then-act patterns for file access and credit charging
Usetransaction=Truefor Redis pipelines to ensure atomicity on multi-step operations
Usemax(0, value)guards for computed values that should never be negative
Keep files under ~300 lines; if a file grows beyond this, split by responsibility (extract helpers, models, or a sub-module into a new file)
Keep functions under ~40 lines; extract named helpers when a function grows longer
...
Files:
autogpt_platform/backend/backend/copilot/sdk/service_test.pyautogpt_platform/backend/backend/copilot/sdk/service.py
autogpt_platform/{backend,autogpt_libs}/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Format Python code with
poetry run format
Files:
autogpt_platform/backend/backend/copilot/sdk/service_test.pyautogpt_platform/backend/backend/copilot/sdk/service.py
autogpt_platform/backend/**/*_test.py
📄 CodeRabbit inference engine (autogpt_platform/backend/AGENTS.md)
autogpt_platform/backend/**/*_test.py: Use pytest with snapshot testing for API responses
Colocate test files with source files using*_test.pynaming convention
Mock at boundaries — mock where the symbol is used, not where it's defined; after refactoring, update mock targets to match new module paths
UseAsyncMockfromunittest.mockfor async functions in tests
When writing tests, use Test-Driven Development (TDD): write failing tests marked with@pytest.mark.xfailbefore implementation, then remove the marker once the implementation is complete
When creating snapshots in tests, usepoetry run pytest path/to/test.py --snapshot-update; always review snapshot changes withgit diffbefore committing
Files:
autogpt_platform/backend/backend/copilot/sdk/service_test.py
🧠 Learnings (16)
📓 Common learnings
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12356
File: autogpt_platform/backend/backend/copilot/constants.py:9-12
Timestamp: 2026-03-10T08:39:22.025Z
Learning: In Significant-Gravitas/AutoGPT PR `#12356`, the `COPILOT_SYNTHETIC_ID_PREFIX = "copilot-"` check in `create_auto_approval_record` (human_review.py) is intentional and safe. The `graph_exec_id` passed to this function comes from server-side `PendingHumanReview` DB records (not from user input); the API only accepts `node_exec_id` from users. Synthetic `copilot-*` IDs are only ever created server-side in `run_block.py`. The prefix skip avoids a DB lookup for a `AgentGraphExecution` record that legitimately does not exist for CoPilot sessions, while `user_id` scoping is enforced at the auth layer and on the resulting auto-approval record.
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12632
File: autogpt_platform/backend/backend/copilot/tools/workspace_files.py:0-0
Timestamp: 2026-04-01T04:17:43.495Z
Learning: In autogpt_platform/backend/backend/copilot/sdk/tool_adapter.py (PR `#12632`, commit 12ae03c), the per-tool `BaseTool.read_only` property approach was removed. Instead, `readOnlyHint=True` (via `ToolAnnotations`) is applied unconditionally to ALL tools — including side-effect tools like `bash_exec` and `write_workspace_file` — to enable fully parallel dispatch by the Anthropic SDK/CLI. Do not flag tools with mutating operations (e.g. save_to_path, write operations) for having `readOnlyHint=True`; this is intentional and E2E validated (3x bash_exec(sleep 3) completed in 3.3s vs 9s sequential).
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12774
File: autogpt_platform/backend/backend/copilot/tools/e2b_sandbox.py:0-0
Timestamp: 2026-04-14T06:34:02.835Z
Learning: In `autogpt_platform/backend/backend/copilot/tools/e2b_sandbox.py`, the `asyncio.wait_for()` retry loop around `AsyncSandbox.create()` (introduced in PR `#12774`) can leak up to `_SANDBOX_CREATE_MAX_RETRIES - 1` (≤2) orphaned E2B sandboxes per hang incident because `wait_for` cancels only the client-side wait while E2B may complete server-side provisioning. With the default `on_timeout="pause"` lifecycle, leaked orphaned sandboxes are **paused** (not killed) when their original `end_at` is reached and persist indefinitely until explicitly killed — there is NO automatic E2B project-level cleanup. Operators must manage these manually or via their own cleanup jobs. The sandbox_id is not accessible from the timed-out coroutine, so recovery via `AsyncSandbox.connect(sandbox_id)` is not possible at timeout. This is an intentionally accepted trade-off; a proper fix is deferred to a follow-up PR. Do NOT flag the retry loop as a blocking issue.
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12773
File: autogpt_platform/backend/backend/blocks/autopilot.py:631-638
Timestamp: 2026-04-14T07:35:11.464Z
Learning: In `autogpt_platform/backend/backend/copilot/executor/utils.py`, `CoPilotExecutionEntry` includes a `permissions: CopilotPermissions | None` field (added in PR `#12773` / commit a0184c87b9). `enqueue_copilot_turn` accepts and serializes this field into the queue entry, `_enqueue_for_recovery` in `autopilot.py` accepts and forwards `permissions` to `enqueue_copilot_turn`, and `_execute_async` in `processor.py` restores `entry.permissions` and passes it into `stream_chat_completion_sdk`/`stream_chat_completion_baseline` via `set_execution_context`. This ensures recovered sub-agent turns respect the same tool/block permission ceiling as the original in-process execution (mirroring `_merge_inherited_permissions`). Do NOT flag recovered turns as losing their permission ceiling — it is now fully propagated through the queue.
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12439
File: autogpt_platform/backend/backend/blocks/autogpt_copilot.py:0-0
Timestamp: 2026-03-16T17:00:02.827Z
Learning: In autogpt_platform/backend/backend/blocks/autogpt_copilot.py, the recursion guard uses two module-level ContextVars: `_copilot_recursion_depth` (tracks current nesting depth) and `_copilot_recursion_limit` (stores the chain-wide ceiling). On the first invocation, `_copilot_recursion_limit` is set to `max_recursion_depth`; nested calls use `min(inherited_limit, max_recursion_depth)`, so they can only lower the cap, never raise it. The entry/exit logic is extracted into module-level helper functions. This is the approved pattern for preventing runaway sub-agent recursion in AutogptCopilotBlock (PR `#12439`, commits 348e9f8e2 and 3b70f61b1).
Learnt from: Pwuts
Repo: Significant-Gravitas/AutoGPT PR: 12284
File: autogpt_platform/frontend/src/app/api/openapi.json:11897-11900
Timestamp: 2026-03-04T23:58:18.476Z
Learning: Repo: Significant-Gravitas/AutoGPT — PR `#12284`
Backend/frontend OpenAPI codegen convention: In backend/api/features/store/model.py, the StoreSubmission and StoreSubmissionAdminView models define submitted_at: datetime | None, changes_summary: str | None, and instructions: str | None with no default. This is intentional to produce “required but nullable” fields in OpenAPI (properties appear in required[] and use anyOf [type, null]). This matches Prisma’s submittedAt DateTime? and changesSummary String?. Do not flag this as a required/nullable mismatch.
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12797
File: autogpt_platform/backend/backend/copilot/sdk/service.py:1991-2021
Timestamp: 2026-04-15T13:44:34.273Z
Learning: In `autogpt_platform/backend/backend/copilot/sdk/service.py` (`_run_stream_attempt`), the pre-create block (PR `#12797`) intentionally does NOT call `state.transcript_builder.append_assistant(...)` when inserting the empty assistant placeholder into `ctx.session.messages`. The transcript is left ending at the `tool_result` entry (N entries) while `message_count` metadata is N+1. This mismatch is benign and deliberate: on the next `--resume`, the SDK sees the transcript ending at `tool_result` and correctly regenerates the assistant response. Pre-appending the assistant turn to the transcript would suppress regeneration while leaving `session.messages[-1].content = ""` permanently (worse outcome). On the gap-fallback path, `transcript_msg_count (N+1) >= msg_count-1 (N)` means no gap is injected for the empty placeholder, which is correct because injecting an empty assistant message as context would mislead the SDK. Do NOT flag this transcript/message_count discrepancy as a bug.
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12701
File: .claude/skills/orchestrate/scripts/verify-complete.sh:120-121
Timestamp: 2026-04-07T18:08:03.548Z
Learning: In Significant-Gravitas/AutoGPT, verify-complete.sh (`.claude/skills/orchestrate/scripts/verify-complete.sh`) uses `commits[-1].committedDate` (not `updatedAt`) to identify stale CHANGES_REQUESTED reviews. This is intentional: `updatedAt` changes on any PR activity (bot comments, label changes, description edits), which would falsely classify a reviewer's CHANGES_REQUESTED as stale — a silent false negative. The `committedDate` edge case (commit created locally before a review but pushed after) only causes a false positive (unnecessary re-brief), which is the safer failure mode. Do not suggest switching to `updatedAt` for this comparison.
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12773
File: autogpt_platform/backend/backend/copilot/pending_messages.py:52-64
Timestamp: 2026-04-14T14:36:25.545Z
Learning: In `autogpt_platform/backend/backend/copilot` (PR `#12773`, commit d7bced0c6): when draining pending messages into `session.messages`, each message's text is sanitized via `strip_user_context_tags` before persistence to prevent user-controlled `<user_context>` injection from bypassing the trusted server-side context prefix. Additionally, if `upsert_chat_session` fails after draining, the drained `PendingMessage` objects are requeued back to Redis to avoid silent message loss. Do NOT flag the drain-then-requeue pattern as redundant — it is the intentional failure-resilience strategy for the pending buffer.
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12440
File: autogpt_platform/backend/backend/copilot/workflow_import/converter.py:0-0
Timestamp: 2026-03-17T10:57:12.953Z
Learning: In Significant-Gravitas/AutoGPT PR `#12440`, `autogpt_platform/backend/backend/copilot/workflow_import/converter.py` was fully rewritten (commit 732960e2d) to no longer make direct LLM/OpenAI API calls. The converter now builds a structured text prompt for AutoPilot/CoPilot instead. There is no `response.choices` access or any direct LLM client usage in this file. Do not flag `response.choices` access or LLM client initialization patterns as issues in this file.
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12173
File: autogpt_platform/backend/backend/copilot/sdk/tool_adapter.py:153-179
Timestamp: 2026-02-20T03:28:06.619Z
Learning: The copilot executor (`backend.copilot.executor`) runs in its own Docker container with Python 3.13, so Python 3.11+ features like `asyncio.timeout()` are available in copilot code even though the general backend pyproject.toml specifies `python = ">=3.10,<3.14"`.
📚 Learning: 2026-04-01T04:17:41.600Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12632
File: autogpt_platform/backend/backend/copilot/tools/workspace_files.py:0-0
Timestamp: 2026-04-01T04:17:41.600Z
Learning: When reviewing AutoGPT Copilot tool implementations, accept that `readOnlyHint=True` (provided via `ToolAnnotations`) may be applied unconditionally to *all* tools—even tools that have side effects (e.g., `bash_exec`, `write_workspace_file`, or other write/save operations). Do **not** flag these tools for having `readOnlyHint=True`; this is intentional to enable fully-parallel dispatch by the Anthropic SDK/CLI and has been E2E validated. Only flag `readOnlyHint` issues if they conflict with the established `ToolAnnotations` behavior (e.g., missing/incorrect propagation relative to the intended annotation mechanism).
Applied to files:
autogpt_platform/backend/backend/copilot/sdk/service_test.pyautogpt_platform/backend/backend/copilot/sdk/service.py
📚 Learning: 2026-02-04T16:49:42.490Z
Learnt from: CR
Repo: Significant-Gravitas/AutoGPT PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-02-04T16:49:42.490Z
Learning: Applies to autogpt_platform/backend/**/test/**/*.py : Use snapshot testing with '--snapshot-update' flag in backend tests when output changes; always review with 'git diff'
Applied to files:
autogpt_platform/backend/backend/copilot/sdk/service_test.py
📚 Learning: 2026-04-13T14:19:19.341Z
Learnt from: Pwuts
Repo: Significant-Gravitas/AutoGPT PR: 12740
File: autogpt_platform/frontend/src/app/api/openapi.json:0-0
Timestamp: 2026-04-13T14:19:19.341Z
Learning: Repo: Significant-Gravitas/AutoGPT — autogpt_platform
When adding new CoPilot tool response models (e.g., ScheduleListResponse, ScheduleDeletedResponse), update backend/api/features/chat/routes.py to include them in the ToolResponseUnion so the frontend’s autogenerated openapi.json dummy export (/api/chat/schema/tool-responses) exposes them for codegen. Do not hand-edit frontend/src/app/api/openapi.json.
Applied to files:
autogpt_platform/backend/backend/copilot/sdk/service_test.py
📚 Learning: 2026-03-17T10:57:12.953Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12440
File: autogpt_platform/backend/backend/copilot/workflow_import/converter.py:0-0
Timestamp: 2026-03-17T10:57:12.953Z
Learning: In Significant-Gravitas/AutoGPT PR `#12440`, `autogpt_platform/backend/backend/copilot/workflow_import/converter.py` was fully rewritten (commit 732960e2d) to no longer make direct LLM/OpenAI API calls. The converter now builds a structured text prompt for AutoPilot/CoPilot instead. There is no `response.choices` access or any direct LLM client usage in this file. Do not flag `response.choices` access or LLM client initialization patterns as issues in this file.
Applied to files:
autogpt_platform/backend/backend/copilot/sdk/service_test.py
📚 Learning: 2026-04-01T04:17:38.279Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12632
File: autogpt_platform/backend/backend/copilot/sdk/tool_adapter.py:530-535
Timestamp: 2026-04-01T04:17:38.279Z
Learning: In `autogpt_platform/backend/backend/copilot/sdk/tool_adapter.py`, the `ToolAnnotations(readOnlyHint=True)` annotation (stored as `_PARALLEL_ANNOTATION`) is intentionally applied to ALL registered MCP tools — including E2B write/edit tools (e.g., `write_file`, `edit_file`). This is a parallel-dispatch hint to the Claude Agent SDK CLI, not a semantic read-only contract. The `_READ_ONLY_E2B_TOOLS` set was dead code and was removed in commit `12ae03c`; the constant was renamed from `_READONLY_ANNOTATION` to `_PARALLEL_ANNOTATION` in commit `c88ca88` to avoid confusion. Do not flag this as a correctness issue.
Applied to files:
autogpt_platform/backend/backend/copilot/sdk/service_test.pyautogpt_platform/backend/backend/copilot/sdk/service.py
📚 Learning: 2026-04-08T17:28:23.439Z
Learnt from: CR
Repo: Significant-Gravitas/AutoGPT PR: 0
File: autogpt_platform/backend/AGENTS.md:0-0
Timestamp: 2026-04-08T17:28:23.439Z
Learning: Applies to autogpt_platform/backend/**/*_test.py : When creating snapshots in tests, use `poetry run pytest path/to/test.py --snapshot-update`; always review snapshot changes with `git diff` before committing
Applied to files:
autogpt_platform/backend/backend/copilot/sdk/service_test.py
📚 Learning: 2026-02-26T17:02:22.448Z
Learnt from: Pwuts
Repo: Significant-Gravitas/AutoGPT PR: 12211
File: .pre-commit-config.yaml:160-179
Timestamp: 2026-02-26T17:02:22.448Z
Learning: Keep the pre-commit hook pattern broad for autogpt_platform/backend to ensure OpenAPI schema changes are captured. Do not narrow to backend/api/ alone, since the generated schema depends on Pydantic models across multiple directories (backend/data/, backend/blocks/, backend/copilot/, backend/integrations/, backend/util/). Narrowing could miss schema changes and cause frontend type desynchronization.
Applied to files:
autogpt_platform/backend/backend/copilot/sdk/service_test.pyautogpt_platform/backend/backend/copilot/sdk/service.py
📚 Learning: 2026-03-04T08:04:35.881Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12273
File: autogpt_platform/backend/backend/copilot/tools/workspace_files.py:216-220
Timestamp: 2026-03-04T08:04:35.881Z
Learning: In the AutoGPT Copilot backend, ensure that SVG images are not treated as vision image types by excluding 'image/svg+xml' from INLINEABLE_MIME_TYPES and MULTIMODAL_TYPES in tool_adapter.py; the Claude API supports PNG, JPEG, GIF, and WebP for vision. SVGs (XML text) should be handled via the text path instead, not the vision path.
Applied to files:
autogpt_platform/backend/backend/copilot/sdk/service_test.pyautogpt_platform/backend/backend/copilot/sdk/service.py
📚 Learning: 2026-03-05T15:42:08.207Z
Learnt from: ntindle
Repo: Significant-Gravitas/AutoGPT PR: 12297
File: .claude/skills/backend-check/SKILL.md:14-16
Timestamp: 2026-03-05T15:42:08.207Z
Learning: In Python files under autogpt_platform/backend (recursively), rely on poetry run format to perform formatting (Black + isort) and linting (ruff). Do not run poetry run lint as a separate step after poetry run format, since format already includes linting checks.
Applied to files:
autogpt_platform/backend/backend/copilot/sdk/service_test.pyautogpt_platform/backend/backend/copilot/sdk/service.py
📚 Learning: 2026-03-16T16:35:40.236Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12440
File: autogpt_platform/backend/backend/api/features/workflow_import.py:54-63
Timestamp: 2026-03-16T16:35:40.236Z
Learning: Avoid using the word 'competitor' in public-facing identifiers and text. Use neutral naming for API paths, model names, function names, and UI text. Examples: rename 'CompetitorFormat' to 'SourcePlatform', 'convert_competitor_workflow' to 'convert_workflow', '/competitor-workflow' to '/workflow'. Apply this guideline to files under autogpt_platform/backend and autogpt_platform/frontend.
Applied to files:
autogpt_platform/backend/backend/copilot/sdk/service_test.pyautogpt_platform/backend/backend/copilot/sdk/service.py
📚 Learning: 2026-03-31T15:37:38.626Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12623
File: autogpt_platform/backend/backend/copilot/tools/agent_generator/fixer.py:37-47
Timestamp: 2026-03-31T15:37:38.626Z
Learning: When validating/constructing Anthropic API model IDs in Significant-Gravitas/AutoGPT, allow the hyphen-separated Claude Opus 4.6 model ID `claude-opus-4-6` (it corresponds to `LlmModel.CLAUDE_4_6_OPUS` in `autogpt_platform/backend/backend/blocks/llm.py`). Do NOT require the dot-separated form in Anthropic contexts. Only OpenRouter routing variants should use the dot separator (e.g., `anthropic/claude-opus-4.6`); `claude-opus-4-6` should be treated as correct when passed to Anthropic, and flagged only if it’s used in the OpenRouter path where the dot form is expected.
Applied to files:
autogpt_platform/backend/backend/copilot/sdk/service_test.pyautogpt_platform/backend/backend/copilot/sdk/service.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/sdk/service_test.pyautogpt_platform/backend/backend/copilot/sdk/service.py
📚 Learning: 2026-04-15T13:44:34.273Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12797
File: autogpt_platform/backend/backend/copilot/sdk/service.py:1991-2021
Timestamp: 2026-04-15T13:44:34.273Z
Learning: In `autogpt_platform/backend/backend/copilot/sdk/service.py` (`_run_stream_attempt`), the pre-create block (PR `#12797`) intentionally does NOT call `state.transcript_builder.append_assistant(...)` when inserting the empty assistant placeholder into `ctx.session.messages`. The transcript is left ending at the `tool_result` entry (N entries) while `message_count` metadata is N+1. This mismatch is benign and deliberate: on the next `--resume`, the SDK sees the transcript ending at `tool_result` and correctly regenerates the assistant response. Pre-appending the assistant turn to the transcript would suppress regeneration while leaving `session.messages[-1].content = ""` permanently (worse outcome). On the gap-fallback path, `transcript_msg_count (N+1) >= msg_count-1 (N)` means no gap is injected for the empty placeholder, which is correct because injecting an empty assistant message as context would mislead the SDK. Do NOT flag this transcript/message_count discrepancy as a bug.
Applied to files:
autogpt_platform/backend/backend/copilot/sdk/service.py
📚 Learning: 2026-03-17T06:48:26.471Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12445
File: autogpt_platform/backend/backend/copilot/sdk/service.py:1071-1072
Timestamp: 2026-03-17T06:48:26.471Z
Learning: In Significant-Gravitas/AutoGPT (autogpt_platform), the AI SDK enforces `z.strictObject({type, errorText})` on SSE `StreamError` responses, so additional fields like `retryable: bool` cannot be added to `StreamError` or serialized via `to_sse()`. Instead, retry signaling for transient Anthropic API errors is done via the `COPILOT_RETRYABLE_ERROR_PREFIX` constant prepended to persisted session messages (in `ChatMessage.content`). The frontend detects retryable errors by checking `markerType === "retryable_error"` from `parseSpecialMarkers()` — no SSE schema changes and no string matching on error text. This pattern was established in PR `#12445`, commit 64d82797b.
Applied to files:
autogpt_platform/backend/backend/copilot/sdk/service.py
📚 Learning: 2026-04-14T06:34:02.835Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12774
File: autogpt_platform/backend/backend/copilot/tools/e2b_sandbox.py:0-0
Timestamp: 2026-04-14T06:34:02.835Z
Learning: In `autogpt_platform/backend/backend/copilot/tools/e2b_sandbox.py`, the `asyncio.wait_for()` retry loop around `AsyncSandbox.create()` (introduced in PR `#12774`) can leak up to `_SANDBOX_CREATE_MAX_RETRIES - 1` (≤2) orphaned E2B sandboxes per hang incident because `wait_for` cancels only the client-side wait while E2B may complete server-side provisioning. With the default `on_timeout="pause"` lifecycle, leaked orphaned sandboxes are **paused** (not killed) when their original `end_at` is reached and persist indefinitely until explicitly killed — there is NO automatic E2B project-level cleanup. Operators must manage these manually or via their own cleanup jobs. The sandbox_id is not accessible from the timed-out coroutine, so recovery via `AsyncSandbox.connect(sandbox_id)` is not possible at timeout. This is an intentionally accepted trade-off; a proper fix is deferred to a follow-up PR. Do NOT flag the retry loop as a blocking issue.
Applied to files:
autogpt_platform/backend/backend/copilot/sdk/service.py
🔇 Additional comments (4)
autogpt_platform/backend/backend/copilot/sdk/service_test.py (1)
14-14: Import is appropriate for the internal helper test.Directly importing the private constant here keeps the regression test scoped to the SDK service module.
autogpt_platform/backend/backend/copilot/sdk/service.py (3)
169-176: LGTM: immutable allowlist matches adapter-normalized tool names.
run_agentandbash_execmatch the names stored bySDKResponseAdapterafter MCP prefix stripping, andfrozensetis a good fit for this module-level allowlist.
1971-1998: LGTM: non-exempt timeouts now include useful tool diagnostics.Logging unresolved tool names and truncated call IDs should make future timeout triage much easier while preserving the existing soft-fail path.
1961-1969: Tool-level timeout caps already exist.Both
bash_exec(120s max per command) andrun_agent(300s max per wait) have enforced per-invocation timeout bounds. The exemption from the 10-minute idle watchdog is safe because individual tool calls are bounded—if they complete within their per-call limits, the idle timeout doesn't fire. Resources cannot remain active indefinitely during tool execution itself.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## dev #12841 +/- ##
==========================================
+ Coverage 65.44% 65.46% +0.02%
==========================================
Files 1848 1854 +6
Lines 137413 138066 +653
Branches 14745 14793 +48
==========================================
+ Hits 89925 90384 +459
- Misses 44701 44874 +173
- Partials 2787 2808 +21
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
majdyz
force-pushed
the
fix/copilot-idle-timeout-soft-fail
branch
from
April 17, 2026 23:20
043cc10 to
2148763
Compare
majdyz
changed the title
majdyz
commented
Apr 17, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
autogpt_platform/backend/backend/copilot/sdk/tool_adapter_test.py (1)
436-470: Nit: hoist the tool-class imports to module scope.
TestBaseToolDefaultTimeoutimportsBaseTool,RunAgentTool,RunBlockTool, andContinueRunBlockToolinside each test method. As per coding guidelines ("Use top-level imports only — avoid local/inner imports except for lazy imports of heavy optional dependencies likeopenpyxl"), these should be moved to the top of the file unless there's a collection-time side effect to avoid.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@autogpt_platform/backend/backend/copilot/sdk/tool_adapter_test.py` around lines 436 - 470, Hoist the local imports in TestBaseToolDefaultTimeout to module scope: move the imports of BaseTool, RunAgentTool, RunBlockTool, and ContinueRunBlockTool out of the individual test methods and place them at the top of the test module so the tests reference the top-level symbols instead of performing inner imports; keep test method bodies unchanged and ensure there are no collection-time side effects before moving each import.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@autogpt_platform/backend/backend/copilot/sdk/tool_adapter.py`:
- Around line 318-327: Update _redact_args_for_log to mask sensitive fields by
key before truncating values: detect keys in args (case-insensitive) that match
a small allowlist/regex (e.g., "password", "token", "api_key", "secret",
"authorization", "credential", "email") and replace their values with a fixed
placeholder like "[REDACTED]" (or partially masked form) regardless of length;
then apply the existing truncation logic for remaining string values, keep
json.dumps(default=str) and the 500-char cap, and ensure the masking handles
keys at the top level (and optionally nested dicts) to avoid leaking short
secrets.
---
Nitpick comments:
In `@autogpt_platform/backend/backend/copilot/sdk/tool_adapter_test.py`:
- Around line 436-470: Hoist the local imports in TestBaseToolDefaultTimeout to
module scope: move the imports of BaseTool, RunAgentTool, RunBlockTool, and
ContinueRunBlockTool out of the individual test methods and place them at the
top of the test module so the tests reference the top-level symbols instead of
performing inner imports; keep test method bodies unchanged and ensure there are
no collection-time side effects before moving each import.
🪄 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: d673f6f7-5670-4b43-9683-916a690655e6
📒 Files selected for processing (7)
autogpt_platform/backend/backend/copilot/sdk/service.pyautogpt_platform/backend/backend/copilot/sdk/tool_adapter.pyautogpt_platform/backend/backend/copilot/sdk/tool_adapter_test.pyautogpt_platform/backend/backend/copilot/tools/base.pyautogpt_platform/backend/backend/copilot/tools/continue_run_block.pyautogpt_platform/backend/backend/copilot/tools/run_agent.pyautogpt_platform/backend/backend/copilot/tools/run_block.py
✅ Files skipped from review due to trivial changes (1)
- autogpt_platform/backend/backend/copilot/tools/continue_run_block.py
🚧 Files skipped from review as they are similar to previous changes (1)
- autogpt_platform/backend/backend/copilot/sdk/service.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: check API types
- GitHub Check: Seer Code Review
- GitHub Check: test (3.11)
- GitHub Check: test (3.12)
- GitHub Check: test (3.13)
- GitHub Check: end-to-end tests
- GitHub Check: Analyze (python)
- GitHub Check: Check PR Status
🧰 Additional context used
📓 Path-based instructions (3)
autogpt_platform/backend/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
autogpt_platform/backend/**/*.py: Use Python 3.11 (required; managed by Poetry via pyproject.toml) for backend development
Always run 'poetry run format' (Black + isort) before linting in backend development
Always run 'poetry run lint' (ruff) after formatting in backend development
autogpt_platform/backend/**/*.py: Usepoetry run ...command for executing Python package dependencies
Use top-level imports only — avoid local/inner imports except for lazy imports of heavy optional dependencies likeopenpyxl
Use absolute imports withfrom backend.module import ...for cross-package imports; single-dot relative imports are acceptable for sibling modules within the same package; avoid double-dot relative imports
Do not use duck typing — avoidhasattr/getattr/isinstancefor type dispatch; use typed interfaces/unions/protocols instead
Use Pydantic models over dataclass/namedtuple/dict for structured data
Do not use linter suppressors — no# type: ignore,# noqa,# pyright: ignore; fix the type/code instead
Prefer list comprehensions over manual loop-and-append patterns
Use early return with guard clauses first to avoid deep nesting
Use%sfor deferred interpolation indebuglog statements for efficiency; use f-strings elsewhere for readability (e.g.,logger.debug("Processing %s items", count)vslogger.info(f"Processing {count} items"))
Sanitize error paths by usingos.path.basename()in error messages to avoid leaking directory structure
Be aware of TOCTOU (Time-Of-Check-Time-Of-Use) issues — avoid check-then-act patterns for file access and credit charging
Usetransaction=Truefor Redis pipelines to ensure atomicity on multi-step operations
Usemax(0, value)guards for computed values that should never be negative
Keep files under ~300 lines; if a file grows beyond this, split by responsibility (extract helpers, models, or a sub-module into a new file)
Keep functions under ~40 lines; extract named helpers when a function grows longer
...
Files:
autogpt_platform/backend/backend/copilot/tools/run_agent.pyautogpt_platform/backend/backend/copilot/tools/base.pyautogpt_platform/backend/backend/copilot/sdk/tool_adapter.pyautogpt_platform/backend/backend/copilot/tools/run_block.pyautogpt_platform/backend/backend/copilot/sdk/tool_adapter_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/tools/run_agent.pyautogpt_platform/backend/backend/copilot/tools/base.pyautogpt_platform/backend/backend/copilot/sdk/tool_adapter.pyautogpt_platform/backend/backend/copilot/tools/run_block.pyautogpt_platform/backend/backend/copilot/sdk/tool_adapter_test.py
autogpt_platform/backend/**/*_test.py
📄 CodeRabbit inference engine (autogpt_platform/backend/AGENTS.md)
autogpt_platform/backend/**/*_test.py: Use pytest with snapshot testing for API responses
Colocate test files with source files using*_test.pynaming convention
Mock at boundaries — mock where the symbol is used, not where it's defined; after refactoring, update mock targets to match new module paths
UseAsyncMockfromunittest.mockfor async functions in tests
When writing tests, use Test-Driven Development (TDD): write failing tests marked with@pytest.mark.xfailbefore implementation, then remove the marker once the implementation is complete
When creating snapshots in tests, usepoetry run pytest path/to/test.py --snapshot-update; always review snapshot changes withgit diffbefore committing
Files:
autogpt_platform/backend/backend/copilot/sdk/tool_adapter_test.py
🧠 Learnings (17)
📓 Common learnings
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12356
File: autogpt_platform/backend/backend/copilot/constants.py:9-12
Timestamp: 2026-03-10T08:39:22.025Z
Learning: In Significant-Gravitas/AutoGPT PR `#12356`, the `COPILOT_SYNTHETIC_ID_PREFIX = "copilot-"` check in `create_auto_approval_record` (human_review.py) is intentional and safe. The `graph_exec_id` passed to this function comes from server-side `PendingHumanReview` DB records (not from user input); the API only accepts `node_exec_id` from users. Synthetic `copilot-*` IDs are only ever created server-side in `run_block.py`. The prefix skip avoids a DB lookup for a `AgentGraphExecution` record that legitimately does not exist for CoPilot sessions, while `user_id` scoping is enforced at the auth layer and on the resulting auto-approval record.
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12632
File: autogpt_platform/backend/backend/copilot/tools/workspace_files.py:0-0
Timestamp: 2026-04-01T04:17:43.495Z
Learning: In autogpt_platform/backend/backend/copilot/sdk/tool_adapter.py (PR `#12632`, commit 12ae03c), the per-tool `BaseTool.read_only` property approach was removed. Instead, `readOnlyHint=True` (via `ToolAnnotations`) is applied unconditionally to ALL tools — including side-effect tools like `bash_exec` and `write_workspace_file` — to enable fully parallel dispatch by the Anthropic SDK/CLI. Do not flag tools with mutating operations (e.g. save_to_path, write operations) for having `readOnlyHint=True`; this is intentional and E2E validated (3x bash_exec(sleep 3) completed in 3.3s vs 9s sequential).
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12774
File: autogpt_platform/backend/backend/copilot/tools/e2b_sandbox.py:0-0
Timestamp: 2026-04-14T06:34:02.835Z
Learning: In `autogpt_platform/backend/backend/copilot/tools/e2b_sandbox.py`, the `asyncio.wait_for()` retry loop around `AsyncSandbox.create()` (introduced in PR `#12774`) can leak up to `_SANDBOX_CREATE_MAX_RETRIES - 1` (≤2) orphaned E2B sandboxes per hang incident because `wait_for` cancels only the client-side wait while E2B may complete server-side provisioning. With the default `on_timeout="pause"` lifecycle, leaked orphaned sandboxes are **paused** (not killed) when their original `end_at` is reached and persist indefinitely until explicitly killed — there is NO automatic E2B project-level cleanup. Operators must manage these manually or via their own cleanup jobs. The sandbox_id is not accessible from the timed-out coroutine, so recovery via `AsyncSandbox.connect(sandbox_id)` is not possible at timeout. This is an intentionally accepted trade-off; a proper fix is deferred to a follow-up PR. Do NOT flag the retry loop as a blocking issue.
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12773
File: autogpt_platform/backend/backend/blocks/autopilot.py:631-638
Timestamp: 2026-04-14T07:35:11.464Z
Learning: In `autogpt_platform/backend/backend/copilot/executor/utils.py`, `CoPilotExecutionEntry` includes a `permissions: CopilotPermissions | None` field (added in PR `#12773` / commit a0184c87b9). `enqueue_copilot_turn` accepts and serializes this field into the queue entry, `_enqueue_for_recovery` in `autopilot.py` accepts and forwards `permissions` to `enqueue_copilot_turn`, and `_execute_async` in `processor.py` restores `entry.permissions` and passes it into `stream_chat_completion_sdk`/`stream_chat_completion_baseline` via `set_execution_context`. This ensures recovered sub-agent turns respect the same tool/block permission ceiling as the original in-process execution (mirroring `_merge_inherited_permissions`). Do NOT flag recovered turns as losing their permission ceiling — it is now fully propagated through the queue.
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12439
File: autogpt_platform/backend/backend/blocks/autogpt_copilot.py:0-0
Timestamp: 2026-03-16T17:00:02.827Z
Learning: In autogpt_platform/backend/backend/blocks/autogpt_copilot.py, the recursion guard uses two module-level ContextVars: `_copilot_recursion_depth` (tracks current nesting depth) and `_copilot_recursion_limit` (stores the chain-wide ceiling). On the first invocation, `_copilot_recursion_limit` is set to `max_recursion_depth`; nested calls use `min(inherited_limit, max_recursion_depth)`, so they can only lower the cap, never raise it. The entry/exit logic is extracted into module-level helper functions. This is the approved pattern for preventing runaway sub-agent recursion in AutogptCopilotBlock (PR `#12439`, commits 348e9f8e2 and 3b70f61b1).
Learnt from: Pwuts
Repo: Significant-Gravitas/AutoGPT PR: 12284
File: autogpt_platform/frontend/src/app/api/openapi.json:11897-11900
Timestamp: 2026-03-04T23:58:18.476Z
Learning: Repo: Significant-Gravitas/AutoGPT — PR `#12284`
Backend/frontend OpenAPI codegen convention: In backend/api/features/store/model.py, the StoreSubmission and StoreSubmissionAdminView models define submitted_at: datetime | None, changes_summary: str | None, and instructions: str | None with no default. This is intentional to produce “required but nullable” fields in OpenAPI (properties appear in required[] and use anyOf [type, null]). This matches Prisma’s submittedAt DateTime? and changesSummary String?. Do not flag this as a required/nullable mismatch.
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12797
File: autogpt_platform/backend/backend/copilot/sdk/service.py:1991-2021
Timestamp: 2026-04-15T13:44:34.273Z
Learning: In `autogpt_platform/backend/backend/copilot/sdk/service.py` (`_run_stream_attempt`), the pre-create block (PR `#12797`) intentionally does NOT call `state.transcript_builder.append_assistant(...)` when inserting the empty assistant placeholder into `ctx.session.messages`. The transcript is left ending at the `tool_result` entry (N entries) while `message_count` metadata is N+1. This mismatch is benign and deliberate: on the next `--resume`, the SDK sees the transcript ending at `tool_result` and correctly regenerates the assistant response. Pre-appending the assistant turn to the transcript would suppress regeneration while leaving `session.messages[-1].content = ""` permanently (worse outcome). On the gap-fallback path, `transcript_msg_count (N+1) >= msg_count-1 (N)` means no gap is injected for the empty placeholder, which is correct because injecting an empty assistant message as context would mislead the SDK. Do NOT flag this transcript/message_count discrepancy as a bug.
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12701
File: .claude/skills/orchestrate/scripts/verify-complete.sh:120-121
Timestamp: 2026-04-07T18:08:03.548Z
Learning: In Significant-Gravitas/AutoGPT, verify-complete.sh (`.claude/skills/orchestrate/scripts/verify-complete.sh`) uses `commits[-1].committedDate` (not `updatedAt`) to identify stale CHANGES_REQUESTED reviews. This is intentional: `updatedAt` changes on any PR activity (bot comments, label changes, description edits), which would falsely classify a reviewer's CHANGES_REQUESTED as stale — a silent false negative. The `committedDate` edge case (commit created locally before a review but pushed after) only causes a false positive (unnecessary re-brief), which is the safer failure mode. Do not suggest switching to `updatedAt` for this comparison.
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12773
File: autogpt_platform/backend/backend/copilot/pending_messages.py:52-64
Timestamp: 2026-04-14T14:36:25.545Z
Learning: In `autogpt_platform/backend/backend/copilot` (PR `#12773`, commit d7bced0c6): when draining pending messages into `session.messages`, each message's text is sanitized via `strip_user_context_tags` before persistence to prevent user-controlled `<user_context>` injection from bypassing the trusted server-side context prefix. Additionally, if `upsert_chat_session` fails after draining, the drained `PendingMessage` objects are requeued back to Redis to avoid silent message loss. Do NOT flag the drain-then-requeue pattern as redundant — it is the intentional failure-resilience strategy for the pending buffer.
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12440
File: autogpt_platform/backend/backend/copilot/workflow_import/converter.py:0-0
Timestamp: 2026-03-17T10:57:12.953Z
Learning: In Significant-Gravitas/AutoGPT PR `#12440`, `autogpt_platform/backend/backend/copilot/workflow_import/converter.py` was fully rewritten (commit 732960e2d) to no longer make direct LLM/OpenAI API calls. The converter now builds a structured text prompt for AutoPilot/CoPilot instead. There is no `response.choices` access or any direct LLM client usage in this file. Do not flag `response.choices` access or LLM client initialization patterns as issues in this file.
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12173
File: autogpt_platform/backend/backend/copilot/sdk/tool_adapter.py:153-179
Timestamp: 2026-02-20T03:28:06.619Z
Learning: The copilot executor (`backend.copilot.executor`) runs in its own Docker container with Python 3.13, so Python 3.11+ features like `asyncio.timeout()` are available in copilot code even though the general backend pyproject.toml specifies `python = ">=3.10,<3.14"`.
📚 Learning: 2026-04-01T04:17:41.600Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12632
File: autogpt_platform/backend/backend/copilot/tools/workspace_files.py:0-0
Timestamp: 2026-04-01T04:17:41.600Z
Learning: When reviewing AutoGPT Copilot tool implementations, accept that `readOnlyHint=True` (provided via `ToolAnnotations`) may be applied unconditionally to *all* tools—even tools that have side effects (e.g., `bash_exec`, `write_workspace_file`, or other write/save operations). Do **not** flag these tools for having `readOnlyHint=True`; this is intentional to enable fully-parallel dispatch by the Anthropic SDK/CLI and has been E2E validated. Only flag `readOnlyHint` issues if they conflict with the established `ToolAnnotations` behavior (e.g., missing/incorrect propagation relative to the intended annotation mechanism).
Applied to files:
autogpt_platform/backend/backend/copilot/tools/run_agent.pyautogpt_platform/backend/backend/copilot/tools/base.pyautogpt_platform/backend/backend/copilot/sdk/tool_adapter.pyautogpt_platform/backend/backend/copilot/tools/run_block.pyautogpt_platform/backend/backend/copilot/sdk/tool_adapter_test.py
📚 Learning: 2026-02-26T17:02:22.448Z
Learnt from: Pwuts
Repo: Significant-Gravitas/AutoGPT PR: 12211
File: .pre-commit-config.yaml:160-179
Timestamp: 2026-02-26T17:02:22.448Z
Learning: Keep the pre-commit hook pattern broad for autogpt_platform/backend to ensure OpenAPI schema changes are captured. Do not narrow to backend/api/ alone, since the generated schema depends on Pydantic models across multiple directories (backend/data/, backend/blocks/, backend/copilot/, backend/integrations/, backend/util/). Narrowing could miss schema changes and cause frontend type desynchronization.
Applied to files:
autogpt_platform/backend/backend/copilot/tools/run_agent.pyautogpt_platform/backend/backend/copilot/tools/base.pyautogpt_platform/backend/backend/copilot/sdk/tool_adapter.pyautogpt_platform/backend/backend/copilot/tools/run_block.pyautogpt_platform/backend/backend/copilot/sdk/tool_adapter_test.py
📚 Learning: 2026-03-04T08:04:35.881Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12273
File: autogpt_platform/backend/backend/copilot/tools/workspace_files.py:216-220
Timestamp: 2026-03-04T08:04:35.881Z
Learning: In the AutoGPT Copilot backend, ensure that SVG images are not treated as vision image types by excluding 'image/svg+xml' from INLINEABLE_MIME_TYPES and MULTIMODAL_TYPES in tool_adapter.py; the Claude API supports PNG, JPEG, GIF, and WebP for vision. SVGs (XML text) should be handled via the text path instead, not the vision path.
Applied to files:
autogpt_platform/backend/backend/copilot/tools/run_agent.pyautogpt_platform/backend/backend/copilot/tools/base.pyautogpt_platform/backend/backend/copilot/sdk/tool_adapter.pyautogpt_platform/backend/backend/copilot/tools/run_block.pyautogpt_platform/backend/backend/copilot/sdk/tool_adapter_test.py
📚 Learning: 2026-03-04T12:19:39.243Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12279
File: autogpt_platform/backend/backend/copilot/tools/base.py:184-188
Timestamp: 2026-03-04T12:19:39.243Z
Learning: In autogpt_platform/backend/backend/copilot/tools/, ensure that anonymous users always pass user_id=None to tool execution methods. The anon_ prefix (e.g., anon_123) is used only for PostHog/analytics distinct_id and must not be used as an actual user_id. Use a simple truthiness check on user_id (e.g., if user_id: ... else: ... or a dedicated is_authenticated flag) to distinguish anonymous from authenticated users, and review all tool execution call sites within this directory to prevent accidentally forwarding an anon_ user_id to tools.
Applied to files:
autogpt_platform/backend/backend/copilot/tools/run_agent.pyautogpt_platform/backend/backend/copilot/tools/base.pyautogpt_platform/backend/backend/copilot/tools/run_block.py
📚 Learning: 2026-03-31T14:22:26.566Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12622
File: autogpt_platform/backend/backend/copilot/tools/agent_search.py:223-236
Timestamp: 2026-03-31T14:22:26.566Z
Learning: In files under autogpt_platform/backend/backend/copilot/tools/, ensure agent graph enrichment uses the typed Pydantic model `backend.data.graph.Graph` for `AgentInfo.graph` (i.e., `Graph | None`), not `dict[str, Any]`. When enriching with graph data (e.g., `_enrich_agents_with_graph`), prefer calling `graph_db().get_graph(graph_id, version=None, user_id=user_id)` directly to retrieve the typed `Graph` object rather than routing through JSON conversions like `get_agent_as_json()` / `graph_to_json()`.
Applied to files:
autogpt_platform/backend/backend/copilot/tools/run_agent.pyautogpt_platform/backend/backend/copilot/tools/base.pyautogpt_platform/backend/backend/copilot/tools/run_block.py
📚 Learning: 2026-03-05T15:42:08.207Z
Learnt from: ntindle
Repo: Significant-Gravitas/AutoGPT PR: 12297
File: .claude/skills/backend-check/SKILL.md:14-16
Timestamp: 2026-03-05T15:42:08.207Z
Learning: In Python files under autogpt_platform/backend (recursively), rely on poetry run format to perform formatting (Black + isort) and linting (ruff). Do not run poetry run lint as a separate step after poetry run format, since format already includes linting checks.
Applied to files:
autogpt_platform/backend/backend/copilot/tools/run_agent.pyautogpt_platform/backend/backend/copilot/tools/base.pyautogpt_platform/backend/backend/copilot/sdk/tool_adapter.pyautogpt_platform/backend/backend/copilot/tools/run_block.pyautogpt_platform/backend/backend/copilot/sdk/tool_adapter_test.py
📚 Learning: 2026-03-16T16:35:40.236Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12440
File: autogpt_platform/backend/backend/api/features/workflow_import.py:54-63
Timestamp: 2026-03-16T16:35:40.236Z
Learning: Avoid using the word 'competitor' in public-facing identifiers and text. Use neutral naming for API paths, model names, function names, and UI text. Examples: rename 'CompetitorFormat' to 'SourcePlatform', 'convert_competitor_workflow' to 'convert_workflow', '/competitor-workflow' to '/workflow'. Apply this guideline to files under autogpt_platform/backend and autogpt_platform/frontend.
Applied to files:
autogpt_platform/backend/backend/copilot/tools/run_agent.pyautogpt_platform/backend/backend/copilot/tools/base.pyautogpt_platform/backend/backend/copilot/sdk/tool_adapter.pyautogpt_platform/backend/backend/copilot/tools/run_block.pyautogpt_platform/backend/backend/copilot/sdk/tool_adapter_test.py
📚 Learning: 2026-03-31T15:37:38.626Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12623
File: autogpt_platform/backend/backend/copilot/tools/agent_generator/fixer.py:37-47
Timestamp: 2026-03-31T15:37:38.626Z
Learning: When validating/constructing Anthropic API model IDs in Significant-Gravitas/AutoGPT, allow the hyphen-separated Claude Opus 4.6 model ID `claude-opus-4-6` (it corresponds to `LlmModel.CLAUDE_4_6_OPUS` in `autogpt_platform/backend/backend/blocks/llm.py`). Do NOT require the dot-separated form in Anthropic contexts. Only OpenRouter routing variants should use the dot separator (e.g., `anthropic/claude-opus-4.6`); `claude-opus-4-6` should be treated as correct when passed to Anthropic, and flagged only if it’s used in the OpenRouter path where the dot form is expected.
Applied to files:
autogpt_platform/backend/backend/copilot/tools/run_agent.pyautogpt_platform/backend/backend/copilot/tools/base.pyautogpt_platform/backend/backend/copilot/sdk/tool_adapter.pyautogpt_platform/backend/backend/copilot/tools/run_block.pyautogpt_platform/backend/backend/copilot/sdk/tool_adapter_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/tools/run_agent.pyautogpt_platform/backend/backend/copilot/tools/base.pyautogpt_platform/backend/backend/copilot/sdk/tool_adapter.pyautogpt_platform/backend/backend/copilot/tools/run_block.pyautogpt_platform/backend/backend/copilot/sdk/tool_adapter_test.py
📚 Learning: 2026-04-01T04:17:38.279Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12632
File: autogpt_platform/backend/backend/copilot/sdk/tool_adapter.py:530-535
Timestamp: 2026-04-01T04:17:38.279Z
Learning: In `autogpt_platform/backend/backend/copilot/sdk/tool_adapter.py`, the `ToolAnnotations(readOnlyHint=True)` annotation (stored as `_PARALLEL_ANNOTATION`) is intentionally applied to ALL registered MCP tools — including E2B write/edit tools (e.g., `write_file`, `edit_file`). This is a parallel-dispatch hint to the Claude Agent SDK CLI, not a semantic read-only contract. The `_READ_ONLY_E2B_TOOLS` set was dead code and was removed in commit `12ae03c`; the constant was renamed from `_READONLY_ANNOTATION` to `_PARALLEL_ANNOTATION` in commit `c88ca88` to avoid confusion. Do not flag this as a correctness issue.
Applied to files:
autogpt_platform/backend/backend/copilot/tools/base.pyautogpt_platform/backend/backend/copilot/sdk/tool_adapter.pyautogpt_platform/backend/backend/copilot/sdk/tool_adapter_test.py
📚 Learning: 2026-02-20T03:28:06.619Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12173
File: autogpt_platform/backend/backend/copilot/sdk/tool_adapter.py:153-179
Timestamp: 2026-02-20T03:28:06.619Z
Learning: The copilot executor (`backend.copilot.executor`) runs in its own Docker container with Python 3.13, so Python 3.11+ features like `asyncio.timeout()` are available in copilot code even though the general backend pyproject.toml specifies `python = ">=3.10,<3.14"`.
Applied to files:
autogpt_platform/backend/backend/copilot/sdk/tool_adapter.py
📚 Learning: 2026-02-27T10:45:55.700Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12213
File: autogpt_platform/frontend/src/app/(platform)/copilot/tools/RunMCPTool/helpers.tsx:23-24
Timestamp: 2026-02-27T10:45:55.700Z
Learning: As of PR `#12213`, MCP tool response types (MCPToolsDiscoveredResponse, MCPToolOutputResponse) are defined in openapi.json and frontend code in autogpt_platform/frontend/src/app/(platform)/copilot/tools/RunMCPTool/helpers.tsx uses the generated types from `@/app/api/__generated__/`. Other tools like RunBlock still use inline TypeScript interfaces (e.g., BlockDetailsResponse) for SSE stream payloads that are not included in openapi.json schemas. The pattern is tool-specific: use generated types when available in openapi.json, use inline types only when the payload schema is truly SSE-stream-only and not exposed via OpenAPI.
Applied to files:
autogpt_platform/backend/backend/copilot/sdk/tool_adapter.py
📚 Learning: 2026-03-10T08:38:29.078Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12356
File: autogpt_platform/backend/backend/copilot/tools/run_block.py:297-300
Timestamp: 2026-03-10T08:38:29.078Z
Learning: In autogpt_platform/backend/backend/copilot/tools/run_block.py, the auto-approval key for sensitive block HITL review is session-scoped: it uses graph_exec_id (copilot-session-{session_id}) combined with node_id (copilot-node-{block_id}). If the user enables the auto-approve toggle for a CoPilot session, approving a block type in that session automatically approves all future invocations of that same block type within the same session. If the user does not toggle auto-approve (default), each individual invocation requires its own approval. This behavior should be clearly documented in code and comments, and tested to ensure the toggle reliably toggles between session-scoped auto-approval and per-invocation approval.
Applied to files:
autogpt_platform/backend/backend/copilot/tools/run_block.py
📚 Learning: 2026-03-10T08:38:30.834Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12356
File: autogpt_platform/backend/backend/copilot/tools/run_block.py:349-370
Timestamp: 2026-03-10T08:38:30.834Z
Learning: In the AutoGPT CoPilot HITL flow, the review card presents with editable: false, so the input payload cannot be modified. Therefore credentials resolved before is_block_exec_need_review() remain valid and do not need to be recomputed after the review step. Ensure run_block.py uses the original input_data for credential validation and that any credential caching relies on the pre-review input. If there is any logic that would recompute credentials post-review, guard it against input_data mutations and rely on the UI to enforce immutability. This guideline is specific to autogpt_platform/backend/backend/copilot/tools/run_block.py.
Applied to files:
autogpt_platform/backend/backend/copilot/tools/run_block.py
📚 Learning: 2026-04-14T06:34:02.835Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12774
File: autogpt_platform/backend/backend/copilot/tools/e2b_sandbox.py:0-0
Timestamp: 2026-04-14T06:34:02.835Z
Learning: In `autogpt_platform/backend/backend/copilot/tools/e2b_sandbox.py`, the `asyncio.wait_for()` retry loop around `AsyncSandbox.create()` (introduced in PR `#12774`) can leak up to `_SANDBOX_CREATE_MAX_RETRIES - 1` (≤2) orphaned E2B sandboxes per hang incident because `wait_for` cancels only the client-side wait while E2B may complete server-side provisioning. With the default `on_timeout="pause"` lifecycle, leaked orphaned sandboxes are **paused** (not killed) when their original `end_at` is reached and persist indefinitely until explicitly killed — there is NO automatic E2B project-level cleanup. Operators must manage these manually or via their own cleanup jobs. The sandbox_id is not accessible from the timed-out coroutine, so recovery via `AsyncSandbox.connect(sandbox_id)` is not possible at timeout. This is an intentionally accepted trade-off; a proper fix is deferred to a follow-up PR. Do NOT flag the retry loop as a blocking issue.
Applied to files:
autogpt_platform/backend/backend/copilot/sdk/tool_adapter_test.py
📚 Learning: 2026-03-18T14:03:32.534Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12473
File: autogpt_platform/backend/backend/copilot/tools/agent_browser_integration_test.py:86-195
Timestamp: 2026-03-18T14:03:32.534Z
Learning: In Significant-Gravitas/AutoGPT, the integration tests in `autogpt_platform/backend/backend/copilot/tools/agent_browser_integration_test.py` intentionally use real external URLs (example.com, httpbin.org). They are gated with `pytest.mark.skipif(shutil.which("agent-browser") is None, ...)`, so they are automatically skipped in standard CI where agent-browser is not installed. They are designed to be run explicitly inside the Docker environment to verify that system Chromium (AGENT_BROWSER_EXECUTABLE_PATH=/usr/bin/chromium) actually launches and can fetch pages end-to-end. Do not flag the use of real network calls in these tests as a flakiness concern.
Applied to files:
autogpt_platform/backend/backend/copilot/sdk/tool_adapter_test.py
🔇 Additional comments (6)
autogpt_platform/backend/backend/copilot/tools/run_block.py (1)
30-35: LGTM — reasonable opt-out for run_block.The comment justifies the
Noneoverride (sub-AutoPilot multi-turn streams), and this aligns with the_execute_tool_syncbehavior intool_adapter.py(wrapper bypassed whentimeout_seconds is None).autogpt_platform/backend/backend/copilot/tools/run_agent.py (1)
107-112: LGTM — opt-out consistent with agent execution lifecycle.
wait_for_resultis already bounded to 0–300s by the Pydantic validator (Line 74), and the tool returns anexecution_idfor async progress checks, so skipping the SDK-side per-call timeout is correct here.autogpt_platform/backend/backend/copilot/tools/base.py (1)
143-156: LGTM — clear contract for per-tool timeout.Docstring accurately documents the
Noneopt-out and the soft-fail synthetic-result semantics enforced by_execute_tool_sync. Default of 600s is reasonable as a universal ceiling.autogpt_platform/backend/backend/copilot/sdk/tool_adapter.py (2)
292-316: LGTM — synthetic timeout result is well-shaped for downstream consumers.The payload matches the MCP envelope expected by
response_adapter._extract_tool_output(single text block,isError: True), and the message coaches the agent to fall back gracefully. Soft-fail behavior is preserved.
267-280: Addtimeout_secondsoverride toBashExecToolto prevent hard-cancellation of legitimate long-running shell commands.
BashExecToolinherits the default 600-second timeout fromBaseTooland does not override it (onlyrun_agent,run_block, andcontinue_run_blockopt out). This will hard-cancel legitimate shell operations exceeding 10 minutes (e.g., builds, downloads, large data processing) at the tool_adapter level (line 272). If extended execution times are intended, settimeout_secondstoNoneor an appropriate threshold inBashExecTool. The same issue applies toWriteWorkspaceFileToolif large payload transfers can exceed 600 seconds.autogpt_platform/backend/backend/copilot/sdk/tool_adapter_test.py (1)
344-433: LGTM — good coverage of the new timeout surface.Tests cover the four behaviorally important cases: synthetic result shape on timeout, actual coroutine cancellation,
Nonebypass, and fast-path success. Theasyncio.Eventassertion intest_timeout_cancels_tool_coroutineis a nice touch for verifying real cancellation (not just TimeoutError translation).
majdyz
commented
Apr 17, 2026
majdyz
commented
Apr 17, 2026
majdyz
commented
Apr 17, 2026
majdyz
force-pushed
the
fix/copilot-idle-timeout-soft-fail
branch
from
April 17, 2026 23:39
2148763 to
c32a401
Compare
majdyz
changed the title
majdyz
commented
Apr 17, 2026
majdyz
commented
Apr 17, 2026
majdyz
commented
Apr 17, 2026
majdyz
commented
Apr 17, 2026
r3105237511 (validate_agent_graph + fix_agent_graph missing gate hint) - Added the "REQUIRED: call get_agent_building_guide once per session before this tool — this tool will refuse otherwise" sentence to the tool descriptions, matching create_agent / edit_agent so the agent sees the contract upfront instead of bouncing off the gate error. r3105237509 (cancel_sub_session breaking the retention contract) - cancel_sub_session was eagerly unregistering the entry, so a caller that polled again right after cancel got "not found" instead of the "cancelled" terminal record. The module docstring explicitly promises terminal entries stick around briefly for late polls. - Fix: just cancel the task; the task's done-callback sets finished_at, and prune_finished evicts after _TERMINAL_TTL_SECONDS like the other terminal states. Late-poll regression test test_cancelled_entry_is_retained_until_pruned locks this in.
…budget After adding the 'requires get_agent_building_guide' hint to validate + fix descriptions (r3105237511), the total MCP tool schema went 32111 chars / 32000 budget (tool_schema_test::test_total_schema_char_budget). Shortened the hint across all four gated tools (create/edit/validate/fix) from 'REQUIRED: call get_agent_building_guide once per session before this tool — this tool will refuse otherwise.' (≈110 chars) to 'Requires get_agent_building_guide first (refuses otherwise).' (≈56 chars). Same semantics, ~54 chars saved per tool × 4 = ~216 chars total under budget.
Contributor
Author
🧪 Native E2E v5 — after /pr-address loop closedHEAD: What changed since v4 (045f7e7)
Regression table (9/9 green)
API proofs (live against commit efdf657)A (sync): B (async): tool sequence C (gate): /pr-address recap
Env notes
|
Mirror the run_agent / executor-service pattern for sub-AutoPilots.
Instead of spawning an in-process asyncio.Task in the parent worker
(with shield, a process-scoped registry, shutdown-notifier, and a 6h
abandoned-task cap to paper over tab-close/deploy survival), the tool
now enqueues a regular CoPilotExecutionEntry on the existing
copilot_execution RabbitMQ exchange. Any available copilot_executor
worker picks it up and runs collect_copilot_response exactly the same
way user-initiated turns do. The parent tool waits by subscribing to
the shared stream_registry for the sub's ChatSession and returning
on StreamFinish / StreamError / cap.
Why:
- Deploy + crash resilience: RabbitMQ redelivers the job. A 30-min sub
survives a rolling deploy instead of being nuked by the shutdown
notifier.
- Natural load balancing: parallel subs from one user fan out across
workers, not pinned to one event loop.
- Uniform model: a sub is just another copilot turn. No bespoke task
registry, no shield, no abandoned-task cap.
- No cross-process cancellation gymnastics: cancel is a fan-out event
on the existing copilot_cancel exchange; the worker running the sub
notices and finalises.
What:
- Added `permissions: CopilotPermissions | None` field to
CoPilotExecutionEntry + threaded it into the processor's stream_fn
call so the worker applies the inherited filter.
- New `sdk/session_waiter.py` — tiny helper that subscribes to the
stream registry for a session and waits for StreamFinish/StreamError
within a cap, returning a SessionOutcome ("completed" / "failed" /
"running"). Mirrors tools/execution_utils.wait_for_execution.
- Rewrote run_sub_session._execute: create_session → enqueue_copilot_turn
→ wait_for_session_completion → read the sub's last assistant message
for the completed response. sub_session_id is now the sub's
ChatSession id directly (no separate registry handle).
- Rewrote get_sub_session_result._execute: same wait helper, cancel via
enqueue_cancel_task on the existing FANOUT exchange. Ownership check
is re-verified by loading the ChatSession.
- Deleted sub_session_registry.py entirely (dataclass, module-level
dict, done-callback, prune_finished, notify_shutdown_and_cancel_all,
_reset_for_test).
- Removed the notify_shutdown_and_cancel_all hook from the
copilot_executor worker cleanup — redundant under queue-backed
execution.
Tests: rewrote sub_session_test.py against the new seams
(enqueue_copilot_turn, wait_for_session_completion,
stream_registry.create_session, enqueue_cancel_task). 15 tests cover
all the behavioural paths — dry-run inheritance, permissions
propagation, wait_for_result=0 short-circuit, waiter "completed" path
reading the sub's last assistant message, cap clamping, cancel fan-out,
cross-user rejection, already-terminal short-circuit.
wait_for_session_completion leaked the stream_registry listener task on timeout. subscribe_to_session spawns a background XREAD task keyed by the subscriber queue; long-running polls that hit the cap accumulate orphaned listeners that keep polling Redis every 5s. Fix: wrap the drain in try/finally and call unsubscribe_from_session in the finally so every exit path (cap fire, terminal event, caller cancellation) cleans up.
…ee callers Three code paths used to each run their own "start a copilot turn and wait for the result" dance with subtly different shapes: 1. HTTP SSE endpoint → enqueue + subscribe (chat routes) 2. run_sub_session tool → in-process asyncio.Task (pre-PR) 3. AutoPilotBlock (graph block) → collect_copilot_response inline PR #12841's first migration unified (2) onto the copilot_executor queue. This commit finishes the job by moving (3) onto the same queue and extracting the shared primitive all three paths now share. What landed: * New ``sdk/stream_accumulator.py`` exposes ``EventAccumulator``, ``ToolCallEntry``, ``process_event`` — the same fold logic both ``collect.collect_copilot_response`` and the new waiter use to turn ``StreamTextDelta`` / ``StreamToolInput/OutputAvailable`` / ``StreamUsage`` / ``StreamError`` events into a single result. * ``sdk/session_waiter.py`` gets ``SessionResult`` + ``wait_for_session_result`` (aggregates events into SessionResult) alongside the existing ``wait_for_session_completion`` (just status). ``run_copilot_turn_via_queue`` bundles create_session → enqueue → wait into the canonical three-line invocation. * ``run_sub_session._execute`` becomes a thin wrapper around ``run_copilot_turn_via_queue`` plus ownership / dry_run resolution. Aggregated ``SessionResult`` is surfaced directly — no DB round-trip to read the completed response content. * ``get_sub_session_result`` uses ``wait_for_session_result`` instead of the completion-only waiter, plus a short-circuit when the sub's last persisted message is already terminal (common for late polls). Ownership still verified via ``get_chat_session``. * ``AutoPilotBlock.execute_copilot`` replaces its inline ``collect_copilot_response`` call with ``run_copilot_turn_via_queue``. Now benefits from deploy/crash resilience (RabbitMQ redelivery), natural load balancing across copilot_executor workers, and every stream-level feature that hooks into the executor path (e.g. the pending-messages drain in #12737) uniformly — whereas before it silently bypassed them. * Tests: sub_session_test patches the new seams (session_waiter.enqueue_copilot_turn, run_copilot_turn_via_queue, wait_for_session_result) so tool logic is exercised without RabbitMQ / Redis. 15 tests cover outcome paths, cancel fan-out, already-terminal short-circuit, permissions propagation, cap clamping, and the include_progress+running branch. Deletion: the duplicate ``_ToolCallEntry`` + ``_EventAccumulator`` + ``_process_event`` in collect.py are gone — collect.py now imports from the shared module. Net: -60 LOC, +2 new public modules. Three call sites now share one well-tested primitive; future features that hook into the executor path (session_message_queueing #12737, compaction policy changes, etc.) apply uniformly instead of needing duplicated wiring.
github-actions
Bot
added
the
documentation
The tool-card UI prefixes 'Running …' onto the formatted tool name,
so run_sub_session read as 'Running Run sub session' (double verb)
and run_block read as 'Running Run block'. Added a specific-case
display-name map so the sub-AutoPilot tools show as 'Sub-AutoPilot' /
'Sub-AutoPilot result', run_block shows as 'Action' (matching the
builder UI's own naming), and the rest of the run_* family just drops
the redundant 'run_' prefix ('Running Agent' instead of 'Running Run
agent'). Helpers test covers the new cases.
majdyz
changed the title
majdyz
changed the title
|
|
…ume race Two small correctness fixes on top of the queue migration. 1. SubAgentRecursionError now inherits BlockExecutionError instead of RuntimeError. Recursion-limit-exceeded is a known, handled block-level failure with a specific cause — BlockExecutionError puts it in the right branch of the block framework's exception hierarchy (known handled error, with block_name + block_id context) instead of getting wrapped in BlockUnknownError if it ever propagates past the block's inline except. 2. Sentry r3105409601: ``_already_terminal_result``'s short-circuit could return a PRIOR turn's assistant message when a session was being resumed — the new turn hadn't persisted its user message yet, so the "last message is terminal" check saw stale data. Now we first consult ``stream_registry.get_session`` and skip the short- circuit entirely whenever the registry reports a running turn for this session. Regression test: ``test_resume_turn_in_flight_does_not_return_stale``.
majdyz
force-pushed
the
fix/copilot-idle-timeout-soft-fail
branch
from
April 18, 2026 15:56
9cc866e to
7476cfe
Compare
github-project-automation
Bot
moved this from 🆕 Needs initial review
to ✅ Done
in AutoGPT development kanban
majdyz
pushed a commit
that referenced
this pull request
Apr 18, 2026
Resolves merge conflicts with #12841 (unified session execution via run_copilot_turn_via_queue) and moves the queue-on-busy fallback into that shared primitive so run_sub_session, the AutoPilot block, and any future caller that targets an existing session_id all inherit it: - session_waiter.SessionResult gains queued/pending_buffer_length - session_waiter.SessionOutcome gains "queued" - run_copilot_turn_via_queue short-circuits when is_turn_in_flight returns True: push to pending buffer, return ("queued", ...) without creating a stream registry session or enqueueing a new turn - SubSessionStatusResponse.status gains "queued"; run_sub_session's response_from_outcome renders a clear queued-state message with the pending-buffer depth and a link to watch live - AutoPilotBlock.execute_copilot surfaces a queued response text when result.queued is True, empty tool_calls/history Net: a single queue primitive reused by (a) POST /stream (server busy-branch), (b) run_sub_session copilot tool, (c) AutoPilot block. Handing any of them a session_id with a live turn now queues the follow-up instead of racing the cluster lock.
majdyz
pushed a commit
that referenced
this pull request
Apr 18, 2026
Resolves merge conflicts with #12841 (unified session execution via run_copilot_turn_via_queue) and moves the queue-on-busy fallback into that shared primitive so run_sub_session, the AutoPilot block, and any future caller that targets an existing session_id all inherit it: - session_waiter.SessionResult gains queued/pending_buffer_length - session_waiter.SessionOutcome gains "queued" - run_copilot_turn_via_queue short-circuits when is_turn_in_flight returns True: push to pending buffer, return ("queued", ...) without creating a stream registry session or enqueueing a new turn - SubSessionStatusResponse.status gains "queued"; run_sub_session's response_from_outcome renders a clear queued-state message with the pending-buffer depth and a link to watch live - AutoPilotBlock.execute_copilot surfaces a queued response text when result.queued is True, empty tool_calls/history Net: a single queue primitive reused by (a) POST /stream (server busy-branch), (b) run_sub_session copilot tool, (c) AutoPilot block. Handing any of them a session_id with a live turn now queues the follow-up instead of racing the cluster lock.
majdyz
added a commit
that referenced
this pull request
Apr 18, 2026
Resolves merge conflicts with #12841 (unified session execution via run_copilot_turn_via_queue) and moves the queue-on-busy fallback into that shared primitive so run_sub_session, the AutoPilot block, and any future caller that targets an existing session_id all inherit it: - session_waiter.SessionResult gains queued/pending_buffer_length - session_waiter.SessionOutcome gains "queued" - run_copilot_turn_via_queue short-circuits when is_turn_in_flight returns True: push to pending buffer, return ("queued", ...) without creating a stream registry session or enqueueing a new turn - SubSessionStatusResponse.status gains "queued"; run_sub_session's response_from_outcome renders a clear queued-state message with the pending-buffer depth and a link to watch live - AutoPilotBlock.execute_copilot surfaces a queued response text when result.queued is True, empty tool_calls/history Net: a single queue primitive reused by (a) POST /stream (server busy-branch), (b) run_sub_session copilot tool, (c) AutoPilot block. Handing any of them a session_id with a live turn now queues the follow-up instead of racing the cluster lock.
majdyz
added a commit
that referenced
this pull request
Apr 18, 2026
…b_session + AutoPilot block) (#12737) ## Why Users and tools can target a copilot session that already has a turn running. Before this PR there was no uniform behaviour for that case — the UI manually routed to a separate queue endpoint, `run_sub_session` and the AutoPilot block raced the cluster lock, and in-turn follow-ups only reached the model at turn-end via auto-continue. Outcome: dropped messages, duplicate tool rows, missed mid-turn intent, latent correctness bugs in block execution. ## What A single "message arrived → turn already running?" primitive, shared by every caller: 1. **POST `/stream`** (UI chat): self-defensive. Session idle → SSE as today; session busy → `202 application/json` with `{buffer_length, max_buffer_length, turn_in_flight}`. The deprecated `POST /messages/pending` endpoint is removed (`GET /messages/pending` peek stays). 2. **`run_copilot_turn_via_queue`** (shared primitive from #12841, used by `run_sub_session` + `AutoPilotBlock`): gains the same busy-check. Busy session → push to pending buffer, return `("queued", SessionResult(queued=True, pending_buffer_length=N))` without creating a stream registry session or enqueueing a RabbitMQ job. All callers inherit queueing. 3. **Mid-turn delivery**: drained follow-ups are attached to every tool_result's `additionalContext` via the SDK's `PostToolUse` hook — covers both MCP and built-in tools (WebSearch/Read/Agent/etc.), not just `run_block`. Claude reads the queued text on the next LLM round of the same turn. 4. **UI observability**: chips promote to a proper user bubble at the correct chronological position (after the tool_result row that consumed them). Auto-continue handles end-of-turn drainage; mid-turn backend poll handles the tool-boundary drainage path. ## How **Data plane** - `backend/copilot/pending_messages.py` — Redis list per session (LPOP-count for atomic drain), TTL, fire-and-forget pub/sub notify. MAX 10 per session. - `backend/copilot/pending_message_helpers.py` — `is_turn_in_flight`, `queue_user_message`, `drain_and_format_for_injection`, `persist_pending_as_user_rows` (shared persist+rollback used by both baseline and SDK paths). - `backend/data/redis_helpers.py` — centralised `incr_with_ttl`, `capped_rpush`, `hash_compare_and_set`; every Lua script and pipeline atomicity lives in one place. **Injection sites** - `backend/copilot/sdk/security_hooks.py::post_tool_use_hook` — drains + returns `additionalContext`. Single hook covers built-in + MCP tools. - `backend/copilot/sdk/service.py` — `StreamToolOutputAvailable` dispatch persists the drained follow-up as a real user row right after the tool_result (UI bubble at the right index). `state.midturn_user_rows` keeps the CLI upload watermark honest. - `backend/copilot/baseline/service.py` — same drain at round boundaries, uses the shared `persist_pending_as_user_rows` helper so baseline + SDK code paths don't diverge. **Dispatch** - `backend/copilot/sdk/session_waiter.py::run_copilot_turn_via_queue` — `is_turn_in_flight` short-circuit; `SessionResult` gains `queued` + `pending_buffer_length`; `SessionOutcome` gains `"queued"`. - `backend/api/features/chat/routes.py::stream_chat_post` — busy-check returns 202 with `QueuePendingMessageResponse`; `POST /messages/pending` deleted. - `backend/copilot/tools/run_sub_session.py` / `models.py` — `SubSessionStatusResponse.status` gains `"queued"`; `response_from_outcome` renders a clear queued-state message with the pending-buffer depth and a link to watch live. - `backend/blocks/autopilot.py::execute_copilot` — surfaces queued state as descriptive response text + empty `tool_calls`/history when `result.queued`. **Frontend** - `src/app/(platform)/copilot/useCopilotPendingChips.ts` — hook owning the chip lifecycle: backend peek on session load, auto-continue promotion when a second assistant id appears, mid-turn poll that promotes when the backend count drops. - `src/app/(platform)/copilot/useHydrateOnStreamEnd.ts` — force-hydrate-waits-for-fresh-reference dance extracted. - `src/app/(platform)/copilot/helpers/stripReplayPrefix.ts` — pure function with drop / strip / streaming-catch-up cases + helper decomposition. - `src/app/(platform)/copilot/helpers/makePromotedBubble.ts` — one-line helper for the promoted bubble shape. - `src/app/(platform)/copilot/helpers/queueFollowUpMessage.ts` — thin `fetch` wrapper for the 202 path (AI SDK's `useChat` fetcher only handles SSE, so we can't reuse `sendMessage` for the queued response). ## Test plan Backend unit + integration (`poetry run pytest backend/copilot backend/api/features/chat`): - [x] 107 tests pass — pending buffer, drain helpers, routes, session_waiter queue branch, run_sub_session outcome rendering, autopilot block - [x] New `session_waiter_test.py` proves the queue branch short-circuits `stream_registry.create_session` + `enqueue_copilot_turn` - [x] Mid-turn persist has a rollback-and-re-queue path tested for when `session.messages` persist silently fails to back-fill sequences Frontend unit (`pnpm vitest run`): - [x] 630 tests pass incl. 22 new for extracted helpers + hooks - [x] Frontend coverage on touched copilot files: 91%+ (patch 87.37%) Manual (once merged): - [ ] Queue two chips while a tool is running; Claude acknowledges both on the next round, UI shows bubbles in typing order after the tool output - [ ] Hand AutoPilot block an existing session_id that has a live turn; block returns queued status, in-flight turn drains the message on its next round - [ ] `run_sub_session` against a busy sub — status=`queued`, `sub_autopilot_session_link` lets user watch live --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
itsababseh
added a commit
that referenced
this pull request
Jun 15, 2026
## AutoPilot Scheduling, New Design & Out of Beta Changelog covering platform versions `v0.6.59` through `v0.6.63` (May 7 – June 10, 2026). ### Featured sections - **AutoPilot major upgrades** — native scheduling (#13190), self-distilled skills registry (#13195), message queuing (#12841) - **New login & signup** — animated panel, aurora, integrations marquee (#13169) - **Subscriptions out of beta** — plans & payments fully live (#12935) - **Settings rebuilt + profile dropdown** — cleaner layout, integrations tab, quick-action menu (#13138, #12976) ### Improvements listed (not featured) - Trigger On Anything (#12740) - Export Chat as Markdown (#13070) - Auto-open artifact panel (#12997) - Slack block (#13008) - Cost breakdown in briefing panel (#13129) - Session sidebar pagination (#13128) - Faster first response in AutoPilot (#12828) ### Files changed - `docs/platform/changelog/may-7-june-10-2026.md` — new changelog page - `docs/platform/.gitbook/assets/` — 5 new hero images - `docs/platform/SUMMARY.md` — new entry at top - `docs/platform/changelog/README.md` — new row at top of table
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 / What / How
Why: the 10-min stream-level idle timeout was killing legitimate long-running tool calls — notably sub-AutoPilot runs via
run_block(AutoPilotBlock), which routinely take 15–45 min. The symptom users saw was"A tool call appears to be stuck"even though AutoPilot was actively working. A second long-standing rough edge was shipped alongside: agents often skippedget_agent_building_guidewhen generating agent JSON, producing schemas that failed validation and burned turns on auto-fix loops.What: three threaded pieces.
Async sub-AutoPilot via
run_sub_session. New copilot tool that delegates a task to a fresh (or resumed) sub-AutoPilot, and its companionget_sub_session_resultfor polling/cancelling. The agent starts withrun_sub_session(prompt, wait_for_result≤300s)and, if the sub isn't done inside the cap, receives a handle + polls viaget_sub_session_result(wait_if_running≤300s). No single MCP call ever blocks the stream for more than 5 min, so the 10-min stream-idle timer stays simple and effective (derived asMAX_TOOL_WAIT_SECONDS * 2).Queue-backed copilot turn dispatch — one code path for all three callers.
run_sub_sessionenqueues aCoPilotExecutionEntryon the existingcopilot_executionexchange instead of spawning an in-processasyncio.Task.AutoPilotBlock.execute_copilot(graph block) now uses the same queue instead ofcollect_copilot_responseinline.run_copilot_turn_via_queue→create_session→enqueue_copilot_turn→wait_for_session_result. The event-aggregation logic (EventAccumulator/process_event) is a shared module used by both the direct-stream path and the cross-process waiter./copilot?sessionId=<inner>into any sub or AutoPilot block's session), and every future stream-level feature (pending-messages drain feat(copilot): queue follow-up messages on busy sessions (UI + run_sub_session + AutoPilot block) #12737, compaction policies, etc.) applies uniformly instead of bypassing graph-block sessions.Guide-read gate on agent-generation tools.
create_agent/edit_agent/validate_agent_graph/fix_agent_graphrefuse until the session has calledget_agent_building_guide. The pre-existing soft hint was routinely ignored; the gate makes the dependency enforceable. All four tool descriptions advertise the requirement in one tightened sentence ("Requires get_agent_building_guide first (refuses otherwise).") that stays under the 32000-char schema budget.How:
Queue-backed sub-AutoPilot + AutoPilotBlock
sdk/session_waiter.py— new module.SessionResultdataclass mirrorsCopilotResult.wait_for_session_resultsubscribes tostream_registry, drains events via sharedprocess_event, returns(outcome, result).wait_for_session_completionis the cheaper outcome-only variant.run_copilot_turn_via_queueis the canonical three-step dispatch. Every exit path unsubscribes the listener.sdk/stream_accumulator.py— new module.EventAccumulator,ToolCallEntry,process_eventextracted fromcollect.py. Both the direct-stream and cross-process paths now use the same fold logic.tools/run_sub_session.py/tools/get_sub_session_result.py— rewritten around the shared primitive.sub_session_idis now the sub'sChatSessionid directly (no separate registry handle). Ownership re-verified on every call viaget_chat_session. Cancel viaenqueue_cancel_taskon the existingcopilot_cancelfan-out exchange.blocks/autopilot.py—execute_copilotreplaced its inlinecollect_copilot_responsewithrun_copilot_turn_via_queue.SessionResultcarries response text, tool calls, and token usage back from the worker so no DB round-trip is needed. The block's public I/O contract (inputs, outputs,ToolCallEntryshape) is unchanged.CoPilotExecutionEntrygains apermissions: CopilotPermissions | Nonefield forwarded to the worker'sstream_fnso the sub's capability filter survives the queue hop. The processor passes it through tostream_chat_completion_sdk/stream_chat_completion_baseline.sdk/sub_session_registry.py(module-level dict, done-callback, abandoned-task cap,notify_shutdown_and_cancel_all,_reset_for_test), plus the shutdown-notifier hook incopilot_executor.processor.cleanup— redundant under queue-backed execution.Run_block single-tool cap (3)
tools/helpers.execute_blockcaps block execution atMAX_TOOL_WAIT_SECONDS = 5 minviaasyncio.wait_foraround the generator consumption.copilot_tool_timeout tool=run_block block=… block_id=… input_keys=… user=… session=… cap_s=…(grep-friendly) and returns anErrorResponsethat redirects the LLM torun_agent/run_sub_session._charge_block_creditsis called in afinallyguarded byasyncio.shieldand markedcharge_handledbefore the await so cancel-mid-charge doesn't double-bill and cancel-mid-generator-before-charge still settles via the finally.Guide-read gate
helpers.require_guide_read(session, tool_name)scanssession.messagesfor any prior assistant tool call namedget_agent_building_guide(handles both OpenAI and flat shapes). Applied at the top of_executeincreate_agent,edit_agent,validate_agent_graph,fix_agent_graph. Tool descriptions advertise the requirement.Shared timing constants
MAX_TOOL_WAIT_SECONDS = 5 * 60+STREAM_IDLE_TIMEOUT_SECONDS = 2 * MAX_TOOL_WAIT_SECONDSinconstants.py. Every long-running tool (run_agent,view_agent_output,run_sub_session,get_sub_session_result,run_block) imports from one place; no more hardcoded 300 /10*60literals drifting apart. Stream-idle invariant ("no single tool blocks close to the idle timeout") holds by construction.Frontend
run_sub_session→ "Sub-AutoPilot",get_sub_session_result→ "Sub-AutoPilot result",run_block→ "Action" (matches the builder UI's own naming),run_agent→ "Agent". Fixes the double-verb "Running Run …" phrasing.SubSessionStatusResponse.sub_autopilot_session_linksurfaces/copilot?sessionId=<inner>so users can click into any sub's session from the tool-call card — same pattern asrun_agent'slibrary_agent_link.Changes 🏗️
sdk/session_waiter.py,sdk/stream_accumulator.py,tools/run_sub_session.py,tools/get_sub_session_result.py,tools/sub_session_test.py,tools/agent_guide_gate_test.py.SubSessionStatusResponse,SubSessionProgressSnapshot,SessionResult.require_guide_readintools/helpers.py.permissionsfield onCoPilotExecutionEntry, threaded throughprocessor.py→stream_fn.AUTOPILOT_BLOCK_IDinCOPILOT_EXCLUDED_BLOCK_IDS(run_block can't execute AutoPilotBlock; agents userun_sub_sessioninstead).sdk/sub_session_registry.py, processor shutdown-notifier hook.openapi.jsonfor the new response types; block-docs for the updatedToolNameLiteral.Checklist 📋
For code changes:
poetry run format+pyrightcleanpoetry run app+pnpm dev):run_sub_session(wait_for_result=60)returnsstatus="completed"+sub_autopilot_session_linkinline;run_sub_session(wait_for_result=1)returnsstatus="running"+ handle,get_sub_session_result(wait_if_running=60)observesrunning → completedtransitioncopilot_executorqueue end-to-end (verified via logs: ExecutionManager's AutoPilotBlock node spawned sessionf6de335b-…, a differentCoPilotExecutorworker acquired its cluster lock and ran the SDK stream)create_agentwithout a priorget_agent_building_guidereturns the refusal; agent reads the guide and retries successfully