fix(backend/copilot): shorten RabbitMQ heartbeat + TCP keepalive to prevent zombie consumer - #12855
fix(backend/copilot): shorten RabbitMQ heartbeat + TCP keepalive to prevent zombie consumer#12855Swiftyos wants to merge 2 commits into
Conversation
…tion (#12832) ## Problem The CoPilot system prompt contains a `gh auth status` instruction in the E2B-specific `GitHub CLI` section, but models pattern-match to `connect_integration` from the **Tool Discovery Priority** section — which is where the actual decision to call an external service is made. Because the GitHub auth check lives in a separate, later section, it's not salient at the point of decision-making. This causes the model to call `connect_integration(provider='github')` even when `gh` is already authenticated via `GH_TOKEN`, unnecessarily prompting the user. ## Fix Add a 3-line callout directly inside the **Tool Discovery Priority** section: ``` > 🔑 **GitHub exception:** Before calling `connect_integration` for GitHub, > always run `gh auth status` first. If it shows `Logged in`, proceed > directly with `gh`/`git` — no integration connection needed. ``` This places the rule at the exact location where the model decides which tool path to take, preventing the miss. ## Why this works - **Placement over repetition**: The existing instruction isn't wrong — it's just in the wrong spot relative to where the decision is made - **Negative framing**: Explicitly says "before calling `connect_integration`" which directly intercepts the incorrect reflex - **Minimal change**: 4 lines added, zero removed Co-authored-by: Toran Bruce Richards <22963551+Torantulino@users.noreply.github.com>
WalkthroughThis PR enhances RabbitMQ connection stability by introducing configurable heartbeat and TCP keepalive parameters into the RabbitMQ configuration model, applies these settings in the copilot executor, and refines GitHub CLI authentication guidance for the copilot prompting system. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🔍 PR Overlap DetectionThis check compares your PR against all other open PRs targeting the same branch to detect potential merge conflicts early. 🔴 Merge Conflicts DetectedThe following PRs have been tested and will have merge conflicts if merged after this PR. Consider coordinating with the authors.
🟢 Low Risk — File Overlap OnlyThese PRs touch the same files but different sections (click to expand)
Summary: 1 conflict(s), 0 medium risk, 8 low risk (out of 9 PRs with file overlap) Auto-generated on push. Ignores: |
|
|
||
| ### GitHub CLI (`gh`) and git | ||
| - To check if the user has their GitHub account already connected, run `gh auth status`. Always check this before asking them to connect it. | ||
| - To check if the user has their GitHub account already connected, run `gh auth status`. Always check this before running `connect_integration(provider="github")` which will ask the user to connect their GitHub regardless if it's already connected. |
There was a problem hiding this comment.
I made the PR based of master branch so this change is already in master.
There was a problem hiding this comment.
try pull master to dev and push first,
otherwise will the conflict resolve will always be there because the same change is applied on two different squashed commits
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
autogpt_platform/backend/backend/copilot/prompting.py (1)
177-195:⚠️ Potential issue | 🟡 MinorFlag: Changes appear unrelated to PR objectives.
The PR summary describes RabbitMQ heartbeat and TCP keepalive configuration changes to prevent zombie consumers. However, this file contains updates to GitHub CLI authentication guidance for E2B sandbox. There's no apparent connection between the RabbitMQ connection stability work and these prompt changes.
Verify whether:
- This file was included in the PR by mistake, or
- The PR summary is incomplete and should mention the GitHub CLI guidance improvements
Secondary observation: Repetitive guidance
Lines 177 and 183-189 convey the same instruction (check
gh auth statusbefore callingconnect_integration) with the second instance being more emphatic. While repetition can reinforce critical behavior for LLM agents, consider whether both are necessary or if the more detailed MANDATORY section (183-189) alone would suffice.♻️ Optional consolidation
If you prefer to eliminate the duplication, you could remove line 177 and keep only the more detailed MANDATORY section:
### GitHub CLI (`gh`) and git -- To check if the user has their GitHub account already connected, run `gh auth status`. Always check this before running `connect_integration(provider="github")` which will ask the user to connect their GitHub regardless if it's already connected. - If the user has connected their GitHub account, both `gh` and `git` are pre-authenticated — use them directly without any manual login step.The detailed section at lines 183-189 already covers this flow comprehensively.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@autogpt_platform/backend/backend/copilot/prompting.py` around lines 177 - 195, This file appears unrelated to the RabbitMQ/TCP keepalive work in the PR — either remove the GitHub CLI guidance changes from the PR or update the PR summary to explicitly include the GitHub CLI/auth guidance change; if the guidance should remain, remove the redundant instruction that repeats the same rule and keep the detailed MANDATORY block that uses gh auth status and connect_integration(provider="github") (and ensure references to gh auth setup-git and scopes=["repo", "read:org"] remain accurate).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@autogpt_platform/backend/backend/copilot/prompting.py`:
- Around line 177-195: This file appears unrelated to the RabbitMQ/TCP keepalive
work in the PR — either remove the GitHub CLI guidance changes from the PR or
update the PR summary to explicitly include the GitHub CLI/auth guidance change;
if the guidance should remain, remove the redundant instruction that repeats the
same rule and keep the detailed MANDATORY block that uses gh auth status and
connect_integration(provider="github") (and ensure references to gh auth
setup-git and scopes=["repo", "read:org"] remain accurate).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2d421cbd-706b-4921-91ab-205c8de56fe6
📒 Files selected for processing (3)
autogpt_platform/backend/backend/copilot/executor/utils.pyautogpt_platform/backend/backend/copilot/prompting.pyautogpt_platform/backend/backend/data/rabbitmq.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). (7)
- GitHub Check: check API types
- GitHub Check: Seer Code Review
- GitHub Check: test (3.12)
- GitHub Check: test (3.13)
- GitHub Check: test (3.11)
- GitHub Check: end-to-end tests
- GitHub Check: Check PR Status
🧰 Additional context used
📓 Path-based instructions (4)
autogpt_platform/backend/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
autogpt_platform/backend/**/*.py: Use Python 3.11 (required; managed by Poetry via pyproject.toml) for backend development
Always run 'poetry run format' (Black + isort) before linting in backend development
Always run 'poetry run lint' (ruff) after formatting in backend development
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/executor/utils.pyautogpt_platform/backend/backend/data/rabbitmq.pyautogpt_platform/backend/backend/copilot/prompting.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/executor/utils.pyautogpt_platform/backend/backend/data/rabbitmq.pyautogpt_platform/backend/backend/copilot/prompting.py
autogpt_platform/backend/backend/data/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
All data access in backend requires user ID checks; verify this for any 'data/*.py' changes
Files:
autogpt_platform/backend/backend/data/rabbitmq.py
autogpt_platform/**/data/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
For changes touching
data/*.py, validate user ID checks or explain why not needed
Files:
autogpt_platform/backend/backend/data/rabbitmq.py
🧠 Learnings (14)
📓 Common learnings
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: 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: 12796
File: autogpt_platform/backend/backend/api/features/chat/routes.py:504-527
Timestamp: 2026-04-16T12:33:44.990Z
Learning: In `autogpt_platform/backend/backend/api/features/chat/routes.py`, `get_session` (PR `#12796`, commit 3771bfad9c1) closes the TOCTOU race between the initial `stream_registry.get_active_session()` pre-check and `get_chat_messages_paginated()` with a post-check re-verification: after the DB fetch, if `is_initial_load and active_session is not None`, it calls `get_active_session` a second time; if `post_active is None` (stream completed during the window), it resets `from_start=True`, `forward_paginated=True`, and re-fetches messages from sequence 0. Do NOT flag the double `get_active_session` call pattern as redundant — it is the intentional TOCTOU mitigation for pagination direction selection.
Learnt from: 0ubbe
Repo: Significant-Gravitas/AutoGPT PR: 12766
File: autogpt_platform/backend/backend/copilot/stream_registry.py:1175-1193
Timestamp: 2026-04-14T14:45:42.706Z
Learning: In `autogpt_platform/backend/backend/copilot/stream_registry.py`, `disconnect_all_listeners(session_id)` is intentionally pod-local (inspects in-memory `_listener_sessions`) and session-scoped (not subscriber-scoped). It cancels all listener tasks for the session on the current pod only. If the DELETE request hits a different pod, nothing is cancelled on that pod — the XREAD timeout (5 s block + status poll) bounds the worst-case release time. In the rare two-tabs-same-session case both listeners on the same pod would be torn down. A subscriber-scoped cross-pod fan-out (per-listener tokens + Redis pub/sub) is deferred as a follow-up. Do NOT re-flag this as a blocking issue; the limitation is explicitly documented in the function's docstring (PR `#12766`, commit 1f3ebafd5).
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12814
File: autogpt_platform/backend/backend/copilot/model.py:0-0
Timestamp: 2026-04-16T13:28:28.641Z
Learning: In `autogpt_platform/backend/backend/copilot/model.py` (PR `#12814`, commit 259d37083): `append_and_save_message` uses `async with _get_session_lock(session_id)` — the same shared context manager used across the module — which internally acquires `redis-py`'s built-in `Lock` (key `copilot:session_lock:{session_id}`, timeout=10s, blocking_timeout=2s) via an atomic Lua-script. Lock release is also owner-verified via Lua so a slow pod can never delete a lock it no longer holds. On Redis failure the lock is skipped with a warning; the in-function idempotency check (`session.messages[-1].role` and `.content` comparison) still runs as a fallback. Do NOT expect a raw `redis.set(nx=True)` / `redis.delete()` pattern here — that intermediate approach was replaced in commit 259d37083.
📚 Learning: 2026-04-14T07:35:11.464Z
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.
Applied to files:
autogpt_platform/backend/backend/copilot/executor/utils.py
📚 Learning: 2026-03-13T15:49:44.961Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12385
File: autogpt_platform/backend/backend/copilot/rate_limit.py:0-0
Timestamp: 2026-03-13T15:49:44.961Z
Learning: In `autogpt_platform/backend/backend/copilot/rate_limit.py`, the original per-session token window (with a TTL-based reset) was replaced with fixed daily and weekly windows. `resets_at` is now derived from `_daily_reset_time()` (midnight UTC) and `_weekly_reset_time()` (next Monday 00:00 UTC) — deterministic fixed-boundary calculations that require no Redis TTL introspection.
Applied to files:
autogpt_platform/backend/backend/copilot/executor/utils.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/executor/utils.pyautogpt_platform/backend/backend/data/rabbitmq.pyautogpt_platform/backend/backend/copilot/prompting.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/executor/utils.pyautogpt_platform/backend/backend/copilot/prompting.py
📚 Learning: 2026-04-01T04:17:41.600Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12632
File: autogpt_platform/backend/backend/copilot/tools/workspace_files.py:0-0
Timestamp: 2026-04-01T04:17:41.600Z
Learning: When reviewing AutoGPT Copilot tool implementations, accept that `readOnlyHint=True` (provided via `ToolAnnotations`) may be applied unconditionally to *all* tools—even tools that have side effects (e.g., `bash_exec`, `write_workspace_file`, or other write/save operations). Do **not** flag these tools for having `readOnlyHint=True`; this is intentional to enable fully-parallel dispatch by the Anthropic SDK/CLI and has been E2E validated. Only flag `readOnlyHint` issues if they conflict with the established `ToolAnnotations` behavior (e.g., missing/incorrect propagation relative to the intended annotation mechanism).
Applied to files:
autogpt_platform/backend/backend/copilot/executor/utils.pyautogpt_platform/backend/backend/copilot/prompting.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/executor/utils.pyautogpt_platform/backend/backend/data/rabbitmq.pyautogpt_platform/backend/backend/copilot/prompting.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/executor/utils.pyautogpt_platform/backend/backend/data/rabbitmq.pyautogpt_platform/backend/backend/copilot/prompting.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/executor/utils.pyautogpt_platform/backend/backend/data/rabbitmq.pyautogpt_platform/backend/backend/copilot/prompting.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/executor/utils.pyautogpt_platform/backend/backend/data/rabbitmq.pyautogpt_platform/backend/backend/copilot/prompting.py
📚 Learning: 2026-03-15T16:52:15.463Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12426
File: autogpt_platform/backend/backend/copilot/sdk/service.py:0-0
Timestamp: 2026-03-15T16:52:15.463Z
Learning: In Significant-Gravitas/AutoGPT (copilot backend), GitHub tokens (GH_TOKEN / GITHUB_TOKEN) for the `gh` CLI are injected lazily per-command in `autogpt_platform/backend/backend/copilot/tools/bash_exec._execute_on_e2b()` by calling `integration_creds.get_integration_env_vars(user_id)`, not on the global SDK subprocess environment in `sdk/service.py`. This scopes credentials to individual E2B sandbox command invocations and prevents token leakage into tool output streams or uploaded transcripts.
Applied to files:
autogpt_platform/backend/backend/copilot/prompting.py
📚 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/prompting.py
📚 Learning: 2026-03-08T23:28:21.675Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12334
File: docs/integrations/block-integrations/github/repo.md:11-40
Timestamp: 2026-03-08T23:28:21.675Z
Learning: In Significant-Gravitas/AutoGPT, new GitHub block documentation stubs in `docs/integrations/block-integrations/github/` are auto-generated by a docs script with placeholder text (`_Add technical explanation here._` / `_Add practical use case examples here._`) inside `<!-- MANUAL: how_it_works
Applied to files:
autogpt_platform/backend/backend/copilot/prompting.py
📚 Learning: 2026-03-07T07:43:15.754Z
Learnt from: kcze
Repo: Significant-Gravitas/AutoGPT PR: 12328
File: autogpt_platform/frontend/src/app/api/openapi.json:1116-1118
Timestamp: 2026-03-07T07:43:15.754Z
Learning: In Significant-Gravitas/AutoGPT, v2 chat endpoints often declare HTTPBearerJWT at the router level while using Depends(auth.get_user_id) that returns None for unauthenticated users; effective behavior is optional auth. Keep this convention unless doing a repo-wide OpenAPI update; prefer clarifying descriptions over per-operation security changes.
Applied to files:
autogpt_platform/backend/backend/copilot/prompting.py
🔇 Additional comments (3)
autogpt_platform/backend/backend/data/rabbitmq.py (2)
55-77: LGTM — platform detection order is correct.The
elifordering correctly avoidsTCP_KEEPALIVEon Linux (where it has different semantics than on macOS) while still settingTCP_KEEPIDLEthere, and falls back to the macOS name otherwise. Keepalive probe math (60s idle + 3 × 20s = ~2 min to dead-peer detection) matches the PR objective.
104-111: LGTM — backward compatible config extension.Defaults preserve prior behavior (
heartbeat=300,tcp_keepalive=False) for all existingRabbitMQConfigusers, keeping the change appropriately narrow.autogpt_platform/backend/backend/copilot/executor/utils.py (1)
127-135: The heartbeat configuration is safe — work is properly dispatched to a thread pool.The
_handle_run_message()callback executes on pika's IO thread but is lightweight: it parses the message, checks for duplicates, acquires a cluster lock, then submits the actual work (execute_copilot_turn) to aThreadPoolExecutorviaself.executor.submit()(line 384 of manager.py). Acknowledgments and rejections are deferred to the IO thread viaadd_callback_threadsafe()(lines 316–324), so the callback itself never blocks for more than the time needed for these fast operations.With
heartbeat=60(~120s window), the IO thread will continue to exchange heartbeats normally and will not trigger spurious disconnects from long-running message processing, since long-running work occurs in worker threads, not on the IO thread.
Codecov Report❌ Patch coverage is ❌ Your patch check has failed because the patch coverage (33.33%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## dev #12855 +/- ##
==========================================
+ Coverage 65.14% 65.30% +0.16%
==========================================
Files 1831 1866 +35
Lines 135945 139982 +4037
Branches 14534 15028 +494
==========================================
+ Hits 88555 91409 +2854
- Misses 44670 45754 +1084
- Partials 2720 2819 +99
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
majdyz
left a comment
majdyz
left a comment
There was a problem hiding this comment.
Let's make sure we pull master to dev first before merging this pr
|
Question on the Heartbeat semanticsThe PR description says the server drops the connection "within ~2x this interval" but I think that's a misread of pika/RabbitMQ semantics — per the RabbitMQ heartbeats docs, the
That's more aggressive than the description implies. At 60s, any pika IO-thread stall longer than ~60s trips a false-positive disconnect. Realistic causes in prod:
Since the kernel TCP keepalive settings already give a hard detection floor (
Not a blocker — just wondering if 60 was a considered choice vs an "as aggressive as seems reasonable" choice. No correctness risk — just operational churnTo be clear: the Redis So the concern here is strictly operational (reconnect churn, log noise, prefetched-message redistribution under transient stalls) — not correctness. |
|
|
github-project-automation
Bot
moved this to 🆕 Needs initial review
in AutoGPT development kanban
4 participants
Why / What / How
Why: The CoPilot executor's RabbitMQ consumer hung locally with a session stuck in
runningfor ~52 minutes. Investigation found all four sockets from the executor parent process to RabbitMQ inCLOSE_WAIT,copilot_execution_queuesitting at 3 messages with 0 consumers, and_stream_listenerheartbeating forever because nothing ever flippedstatus. Pika's blockingstart_consuming()never saw the server FIN (IO thread starved — likely laptop sleep), so@continuous_retrynever fired and the socket rotted half-closed until the next restart.What: Let each
RabbitMQConfigset a custom AMQPheartbeatand opt into kernel-level TCP keepalive, and wire aggressive values intocreate_copilot_queue_config()so the copilot consumer notices a dead peer within ~2 minutes instead of hours.How:
RabbitMQConfiggainsheartbeat: int = 300(unchanged default) andtcp_keepalive: bool = False(off by default) — existing configs are untouched.SyncRabbitMQ.connect()reads both; whentcp_keepalive=Trueit passes a platform-awaretcp_optionsdict to pika (Linux usesTCP_KEEPIDLE, macOS usesTCP_KEEPALIVE, both getTCP_KEEPINTVL=20,TCP_KEEPCNT=3).AsyncRabbitMQ.connect()readsheartbeattoo; aio_pika doesn't expose the keepalive knobs, so that side stays heartbeat-only.create_copilot_queue_config()overridesheartbeat=60, tcp_keepalive=True. The copilot consumer is the one that blocks instart_consuming()for the full process lifetime, so it's the one that benefits.Graph executor, notifications, and other
RabbitMQConfigusers keep the old 300 s heartbeat with no keepalive — scope is intentionally narrow.Changes 🏗️
backend/data/rabbitmq.py:RabbitMQConfig.heartbeatandRabbitMQConfig.tcp_keepalivefields;_tcp_keepalive_options()helper; both connect paths wired up.backend/copilot/executor/utils.py:create_copilot_queue_config()setsheartbeat=60, tcp_keepalive=Truewith a comment explaining the CLOSE_WAIT incident.Checklist 📋
For code changes:
poetry run ruff checkclean on both filespoetry run black --checkclean on both filespoetry run pytest backend/copilot/executor/utils_test.py— 11 passedcopilot_execution_queuehas 1 consumer viadocker exec rabbitmq rabbitmqctl list_queues name messages consumersdocker restart rabbitmqwhile the executor is idle) and confirm the consumer reconnects within ~2 minutes instead of hanginggraph_execution_queue,immediate_notifications, etc.) are unaffected — their defaults haven't changed