Skip to content

fix(copilot): P0 guardrails, transient retry, and security hardening - #12636

Merged
majdyz merged 51 commits into
devfrom
fix/copilot-p0-cli-internals
Apr 9, 2026
Merged

fix(copilot): P0 guardrails, transient retry, and security hardening#12636
majdyz merged 51 commits into
devfrom
fix/copilot-p0-cli-internals

Conversation

@majdyz

@majdyz majdyz commented Apr 1, 2026

Copy link
Copy Markdown
Contributor

Why

The copilot's Claude Code CLI integration had several production reliability gaps reported from live deployments:

  • No transient retry: 429 rate-limit errors, 5xx server errors, and ECONNRESET connection resets surfaced immediately as failures — there was no retry mechanism.
  • Subagent permission errors: CLI subprocesses wrote temp files to /tmp/claude-0/ which was inaccessible inside E2B sandboxes, causing subagent spawning to report "agent completed" without actually running.
  • Missing security hardening in non-OpenRouter modes: Security env vars (CLAUDE_CODE_DISABLE_CLAUDE_MDS, CLAUDE_CODE_SKIP_PROMPT_HISTORY, CLAUDE_CODE_DISABLE_AUTO_MEMORY, CLAUDE_CODE_DISABLE_NONESSENTIAL_TRAFFIC) were only applied in the OpenRouter path, leaving subscription and direct Anthropic modes unprotected in multi-tenant deployment.
  • No resource guardrails: No per-query budget cap, turn limit, or fallback model meant a single runaway query could burn unlimited tokens/spend.
  • Lossy transcript reconstruction: When no transcript file was available (storage failure or compaction drop), the old code injected a truncated plain-text summary that cut tool results at 500 chars and dropped tool_use/tool_result structural linkage, causing the LLM to lose conversation context.

What

  • SDK guardrails (config.py, sdk/service.py): Added fallback_model (auto-failover on 529 overloaded), max_turns=1000 (runaway prevention), max_budget_usd=100.0 (per-query cost cap). All configurable via env-backed ChatConfig fields.
  • Transient retry (sdk/service.py, constants.py): Exponential backoff (1s, 2s, 4s) for 429/5xx/ECONNRESET errors, retried only when events_yielded == 0 to avoid breaking partial streams. _TRANSIENT_ERROR_PATTERNS extended with status-code-specific patterns to avoid false positives.
  • Workspace isolation (sdk/env.py): CLAUDE_CODE_TMPDIR now set in all auth modes so CLI subprocesses write to the per-session workspace directory rather than /tmp/.
  • Security hardening (sdk/env.py): Security env vars applied uniformly across all three auth modes (subscription, direct Anthropic, OpenRouter) via restructured build_sdk_env().
  • Transcript reconstruction (sdk/service.py): _session_messages_to_transcript() converts ChatMessage.tool_calls and ChatMessage.tool_call_id to proper tool_use/tool_result JSONL blocks for --resume, restoring full structural fidelity.
  • Model normalization refactor (sdk/service.py): _resolve_fallback_model() and _normalize_model_name() extracted to share prefix-stripping and dot→hyphen conversion logic between primary and fallback model resolution.

How it works

Transient retry: _can_retry_transient() checks the retry budget and returns the next backoff delay (or None when exhausted). Retries are gated on events_yielded == 0 — if any events were already streamed to the client, we cannot retry without breaking the SSE stream mid-response. After all retries are exhausted, FRIENDLY_TRANSIENT_MSG is surfaced to the user.

Transcript reconstruction: When --resume has no on-disk session file, _session_messages_to_transcript() builds a JSONL transcript from session.messages, emitting tool_use blocks for assistant tool calls and tool_result blocks (with matching IDs) for their results. This gives Claude CLI the same structural fidelity as an on-disk session — preserving tool call/result pairing that the old plain-text injection lost.

build_sdk_env() restructure: The three auth modes now share a common "epilogue" block that applies workspace isolation and security hardening env vars regardless of which mode is active, eliminating the previous pattern of repeating if sdk_cwd: env["CLAUDE_CODE_TMPDIR"] = sdk_cwd in each branch.

Checklist 📋

For code changes:

  • I have clearly listed my changes in the PR description
  • I have made a test plan
  • I have tested my changes according to the test plan:
    • 729 unit tests passing: env_test.py, p0_guardrails_test.py, retry_scenarios_test.py (incl. integration tests for both transient retry paths), service_test.py, sdk_compat_test.py, response_adapter_test.py
    • E2E tested: live copilot session (API + UI), multi-turn, security env vars verified in all 3 auth modes, guardrail defaults confirmed
    • _session_messages_to_transcript(): 7 unit tests covering empty input, tool_use blocks, tool_result blocks, no truncation (10K chars preserved), parent UUID chain, malformed argument handling

@majdyz
majdyz requested a review from a team as a code owner April 1, 2026 13:05
@majdyz
majdyz requested review from kcze and ntindle and removed request for a team April 1, 2026 13:05
@github-project-automation github-project-automation Bot moved this to 🆕 Needs initial review in AutoGPT development kanban Apr 1, 2026
@github-actions

github-actions Bot commented Apr 1, 2026

Copy link
Copy Markdown
Contributor

This PR targets the master branch but does not come from dev or a hotfix/* branch.

Automatically setting the base branch to dev.

@github-actions github-actions Bot added platform/backend AutoGPT Platform - Back end conflicts Automatically applied to PRs with merge conflicts labels Apr 1, 2026
@github-actions
github-actions Bot changed the base branch from master to dev April 1, 2026 13:06
@github-actions

github-actions Bot commented Apr 1, 2026

Copy link
Copy Markdown
Contributor

This pull request has conflicts with the base branch, please resolve those so we can evaluate the pull request.

@github-actions github-actions Bot added the size/l label Apr 1, 2026
@coderabbitai

coderabbitai Bot commented Apr 1, 2026

Copy link
Copy Markdown
Contributor

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

Adds four Claude agent settings to ChatConfig and extends the copilot SDK service: provider-aware fallback-model resolution, per-session sandbox env injection (CLAUDE_CODE_TMPDIR), propagation of fallback/max_turns/max_budget to the SDK, and transient-error detection with exponential backoff plus mid-stream rollback/retry.

Changes

Cohort / File(s) Summary
Configuration
autogpt_platform/backend/backend/copilot/config.py
Added ChatConfig fields: claude_agent_fallback_model, claude_agent_max_turns, claude_agent_max_budget_usd, and claude_agent_max_transient_retries with defaults and docstrings.
SDK Service Implementation
autogpt_platform/backend/backend/copilot/sdk/service.py
Added _resolve_fallback_model(); injects session sandbox env (e.g., CLAUDE_CODE_TMPDIR) into SDK env; passes fallback_model, max_turns, max_budget_usd into ClaudeAgentOptions; added transient-retry helper with exponential backoff, per-context transient retry budget, mid-stream rollback of session.messages, adapter/usage reset, and transient-aware logging.
Tests
autogpt_platform/backend/backend/copilot/sdk/sdk_compat_test.py
Extended compatibility test to assert ClaudeAgentOptions constructor accepts stderr, fallback_model, max_turns, and max_budget_usd parameters.

Sequence Diagram

sequenceDiagram
    participant Client
    participant SDK_Service as SDK Service
    participant Claude_SDK as Claude SDK
    participant API as Claude/Anthropic API
    participant ErrorHandler as Error Handler

    rect rgba(200,150,255,0.5)
    Client->>SDK_Service: Start streaming request (config, sdk_cwd)
    SDK_Service->>SDK_Service: resolve fallback model, build env (auth, isolation, CLAUDE_CODE_TMPDIR, guards)
    SDK_Service->>Claude_SDK: Init with options (fallback_model, max_turns, max_budget_usd)
    end

    rect rgba(150,200,255,0.5)
    Claude_SDK->>API: Open streaming connection / send request
    API-->>Claude_SDK: Stream events / errors
    Claude_SDK-->>SDK_Service: Emit streamed events
    SDK_Service-->>Client: Yield stream chunks
    end

    rect rgba(255,200,150,0.5)
    alt Transient error detected (429/5xx/ECONNRESET)
        Claude_SDK-->>SDK_Service: Error (transient)
        SDK_Service->>ErrorHandler: classify transient, consult retry budget
        ErrorHandler-->>SDK_Service: backoff delay
        SDK_Service->>SDK_Service: rollback session.messages, yield StreamStatus(backoff)
        SDK_Service->>SDK_Service: sleep exponential backoff, reset adapter/usage state
        SDK_Service->>Claude_SDK: Retry streaming attempt (same context)
    else Non-transient or retries exhausted
        SDK_Service-->>Client: Yield final error/abort
    end
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested reviewers

  • ntindle
  • kcze
  • Bentlybro

Poem

🐰
I hopped through envs and stitched a guard,
Rolled back chat crumbs when the stream ran hard,
Fallbacks snug, budgets counted tight,
Retries timed under the moonlit byte,
A rabbit cheers the SDK yard.

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Title check ✅ Passed The title accurately summarizes the main changes: adding P0 guardrails, transient retry logic, and security hardening to the copilot system.
Description check ✅ Passed The pull request description is directly related to the changeset, detailing specific SDK guardrails, environment variable configurations, retry logic, and transcript handling changes that align with the file modifications.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/copilot-p0-cli-internals

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Comment thread autogpt_platform/backend/backend/copilot/sdk/service.py Outdated
Comment thread autogpt_platform/backend/backend/copilot/sdk/service.py Outdated
Comment thread autogpt_platform/backend/backend/copilot/sdk/service.py
Comment thread autogpt_platform/backend/backend/copilot/sdk/service.py
Comment thread autogpt_platform/backend/backend/copilot/sdk/service.py
Comment thread autogpt_platform/backend/backend/copilot/sdk/service.py Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (1)
autogpt_platform/backend/backend/copilot/sdk/service.py (1)

1393-1401: File and function length exceed guidelines.

This file is ~2240 lines (guideline: ~300) and stream_chat_completion_sdk is ~850 lines (guideline: ~40). The existing extraction of _StreamContext, _RetryState, and _run_stream_attempt shows good progress toward managing complexity.

For a future refactor, consider:

  • Splitting retry logic into a dedicated module (e.g., retry_handler.py)
  • Extracting transcript handling to a separate module
  • Moving E2B setup/teardown to a context manager

This is not blocking for the P0 guardrails fix. As per coding guidelines: "Keep files under ~300 lines" and "Keep functions under ~40 lines".

🤖 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.py` around lines 1393 -
1401, The function stream_chat_completion_sdk is far too large and includes
retry logic, transcript handling, and E2B setup/teardown; extract those
responsibilities into separate modules: create a retry_handler.py that
implements _RetryState and _run_stream_attempt (and exposes a simple retry API
used by stream_chat_completion_sdk), create a transcript_handler.py to
encapsulate transcript creation/updates currently embedded in
stream_chat_completion_sdk, and create an e2b_context.py context manager to
handle E2B setup/teardown; then reduce stream_chat_completion_sdk to an
orchestrator that imports and calls _StreamContext, the retry API, transcript
handler functions, and the E2B context manager so the file and function sizes
fall within guidelines.
🤖 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/service.py`:
- Around line 1690-1698: Update the SDK compatibility test
test_agent_options_accepts_all_our_fields() so its fields_we_use list includes
the three new guardrail option names currently passed into ClaudeAgentOptions in
service.py: "fallback_model", "max_turns", and "max_budget_usd". Locate the
fields_we_use variable in sdk_compat_test.py (used by
test_agent_options_accepts_all_our_fields) and add those three strings to the
list so the test validates all fields referenced in service.py's
ClaudeAgentOptions block.
- Around line 1865-1884: The transient_retries counter is incremented before
comparing to max_transient which causes an off-by-one in allowed retries; if the
intent is "retry up to N times" (N retries after the initial attempt) change the
check from "transient_retries >= max_transient" to "transient_retries >
max_transient" (and apply the same change at the other occurrence around lines
1942-1944) so transient_retries can reach max_transient before stopping; update
the comparisons where log_prefix, events_yielded, transient_retries,
max_transient, FRIENDLY_TRANSIENT_MSG, _append_error_marker, and
ended_with_stream_error are used.

---

Nitpick comments:
In `@autogpt_platform/backend/backend/copilot/sdk/service.py`:
- Around line 1393-1401: The function stream_chat_completion_sdk is far too
large and includes retry logic, transcript handling, and E2B setup/teardown;
extract those responsibilities into separate modules: create a retry_handler.py
that implements _RetryState and _run_stream_attempt (and exposes a simple retry
API used by stream_chat_completion_sdk), create a transcript_handler.py to
encapsulate transcript creation/updates currently embedded in
stream_chat_completion_sdk, and create an e2b_context.py context manager to
handle E2B setup/teardown; then reduce stream_chat_completion_sdk to an
orchestrator that imports and calls _StreamContext, the retry API, transcript
handler functions, and the E2B context manager so the file and function sizes
fall within guidelines.
🪄 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: 8f370d21-b510-4f01-95a6-bb2bb29d0357

📥 Commits

Reviewing files that changed from the base of the PR and between 1750c83 and b8250a6.

📒 Files selected for processing (2)
  • autogpt_platform/backend/backend/copilot/config.py
  • 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). (3)
  • GitHub Check: Seer Code Review
  • GitHub Check: Analyze (javascript-typescript)
  • GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (2)
autogpt_platform/backend/**/*.py

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

autogpt_platform/backend/**/*.py: Use Python 3.11 (required; managed by Poetry via pyproject.toml) for backend development
Always run 'poetry run format' (Black + isort) before linting in backend development
Always run 'poetry run lint' (ruff) after formatting in backend development

Refer to @backend/CLAUDE.md for backend-specific commands, architecture, and development tasks

autogpt_platform/backend/**/*.py: Import only at the top level; no local/inner imports except for lazy imports of heavy optional dependencies like openpyxl
Use absolute imports with from backend.module import ... for cross-package imports; single-dot relative imports (from .sibling import ...) are acceptable for sibling modules within the same package; avoid double-dot relative imports (from ..parent import ...)
Do not use duck typing with hasattr(), getattr(), or isinstance() for type dispatch; use typed interfaces, unions, or protocols instead
Use Pydantic models for structured data instead of dataclasses, namedtuples, or dicts
Do not use linter suppressors; no # type: ignore, # noqa, or # pyright: ignore comments — fix the underlying type/code issue instead
Use list comprehensions instead of manual loop-and-append patterns
Use early return guard clauses to avoid deep nesting
Use %s for deferred interpolation in debug log statements; use f-strings for readability in other log levels (e.g., logger.debug("Processing %s items", count), logger.info(f"Processing {count} items"))
Sanitize error paths using os.path.basename() in error messages to avoid leaking directory structure
Avoid TOCTOU (time-of-check-time-of-use) patterns; do not use check-then-act patterns for file access and credit charging operations
Use Redis pipelines with transaction=True for atomicity on multi-step Redis operations
Use max(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 h...

Files:

  • autogpt_platform/backend/backend/copilot/config.py
  • autogpt_platform/backend/backend/copilot/sdk/service.py
autogpt_platform/**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

Format Python code with poetry run format

Files:

  • autogpt_platform/backend/backend/copilot/config.py
  • autogpt_platform/backend/backend/copilot/sdk/service.py
🧠 Learnings (11)
📓 Common learnings
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.
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: 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: ntindle
Repo: Significant-Gravitas/AutoGPT PR: 12592
File: autogpt_platform/frontend/AGENTS.md:1-3
Timestamp: 2026-03-27T08:39:45.696Z
Learning: In Significant-Gravitas/AutoGPT, Claude is the primary coding agent. AGENTS.md files intentionally retain Claude-specific wording (e.g., "CLAUDE.md - Frontend", "This file provides guidance to Claude Code") even though AGENTS.md is the canonical cross-agent instruction source. Do not flag Claude-specific titles or phrasing in AGENTS.md files as issues.
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: 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: 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.
📚 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/config.py
  • autogpt_platform/backend/backend/copilot/sdk/service.py
📚 Learning: 2026-03-27T08:39:45.696Z
Learnt from: ntindle
Repo: Significant-Gravitas/AutoGPT PR: 12592
File: autogpt_platform/frontend/AGENTS.md:1-3
Timestamp: 2026-03-27T08:39:45.696Z
Learning: In Significant-Gravitas/AutoGPT, Claude is the primary coding agent. AGENTS.md files intentionally retain Claude-specific wording (e.g., "CLAUDE.md - Frontend", "This file provides guidance to Claude Code") even though AGENTS.md is the canonical cross-agent instruction source. Do not flag Claude-specific titles or phrasing in AGENTS.md files as issues.

Applied to files:

  • autogpt_platform/backend/backend/copilot/config.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/config.py
  • autogpt_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/config.py
  • autogpt_platform/backend/backend/copilot/sdk/service.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/config.py
  • autogpt_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/config.py
  • autogpt_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/config.py
  • autogpt_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/config.py
  • autogpt_platform/backend/backend/copilot/sdk/service.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/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
🔇 Additional comments (5)
autogpt_platform/backend/backend/copilot/config.py (1)

118-137: Well-structured guardrail configuration fields.

The new SDK guardrail fields are clearly documented and follow Pydantic conventions. The defaults are reasonable for preventing runaway execution and cost overruns.

Minor consideration: the numeric fields (max_turns, max_budget_usd, max_transient_retries) could benefit from ge=1 or gt=0 validators to prevent misconfiguration with non-positive values, but this is optional since invalid values would likely be caught by the SDK layer.

autogpt_platform/backend/backend/copilot/sdk/service.py (4)

1648-1650: LGTM!

The sdk_cwd parameter is correctly passed to _build_sdk_env(), enabling per-session isolation of CLI temp files and project directories.


1886-1901: Exponential backoff implementation is correct.

The backoff calculation 2 ** (transient_retries - 1) produces 1s, 2s, 4s delays as expected. The StreamStatus message keeps the user informed about retry delays, which is good UX.

Optional improvement: Adding random jitter (e.g., backoff * (0.5 + random.random())) could help avoid thundering herd effects in high-concurrency scenarios, but this is not critical for per-session retries.


1843-1864: Message rollback on retry is correctly implemented.

The pre_attempt_msg_count checkpoint and subsequent rollback (session.messages[:pre_attempt_msg_count]) correctly restore session state before retrying, preventing partial or duplicate messages from accumulating.


556-574: Security hardening approach is sound, but HOME is intentionally NOT set.

The common security guards (disabling .claude.md loading, prompt history, auto-memory, and nonessential traffic) are correctly applied to all authentication modes. However, the code intentionally does NOT set HOME to the workspace—only CLAUDE_CODE_TMPDIR is set. This preserves child process compatibility (git, ssh, npm) that rely on ~/.gitconfig, ~/.ssh/, etc. The container's HOME is already ephemeral, so CLI writes to ~/.claude/projects/ don't persist across sessions anyway. The approach aligns with the principle of scoping credentials and artifacts to individual command invocations rather than global subprocess environment pollution.

			> Likely an incorrect or invalid review comment.

Comment thread autogpt_platform/backend/backend/copilot/sdk/service.py
Comment thread autogpt_platform/backend/backend/copilot/sdk/service.py Outdated
Comment thread autogpt_platform/backend/backend/copilot/sdk/service.py Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 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/service.py`:
- Around line 1966-1993: The transient-error path currently falls through when
_can_retry_transient() returns None, losing the retryable marker; update the
is_transient branch in the streaming/error handling so that when backoff is None
(i.e., retries exhausted) you call _append_error_marker(..., retryable=True)
(using the same message_id/session_id context and
state.adapter/SDKResponseAdapter as present) or otherwise prefix the persisted
ChatMessage.content with COPILOT_RETRYABLE_ERROR_PREFIX before allowing the flow
to continue to the generic sdk_stream_error path; ensure the marker is added for
exhausted 429/5xx/ECONNRESET cases so the frontend retains the "Try again"
affordance.
- Around line 1932-1936: The retry branches reset state.adapter and call
state.usage.reset without reverting changes made to state.transcript_builder by
_run_stream_attempt, causing duplicate/aborted content on retry; fix by
snapshotting state.transcript_builder before each call to _run_stream_attempt
and restoring that snapshot in the retry/except path (i.e., before state.adapter
= SDKResponseAdapter(...) and state.usage.reset()) so the builder is returned to
the pre-attempt state, or alternatively skip uploading/resuming the transcript
after an aborted attempt; apply the same change to the other retry block around
the code at the referenced second spot (the block around lines 1983-1987).
- Around line 1807-1818: The transient-retry guard in _can_retry_transient never
fires because events_yielded is incremented when the transient StreamError is
emitted from _run_stream_attempt, so _TransientErrorHandled paths see
events_yielded>0 and skip retries; fix by excluding internal transient error
events from the duplicate-output count (or deferring emission until retries
exhausted). Concretely: mark the internal transient StreamError emitted by
_run_stream_attempt (or the _TransientErrorHandled path) with a flag like
transient_internal=True (or return it only after max_transient reached), then
update the code that increments events_yielded to skip events with that flag and
adjust _can_retry_transient (and transient_retries handling) to allow backoff
retries (2 ** (transient_retries - 1)) until max_transient is reached; ensure
symbols involved: _can_retry_transient, transient_retries, events_yielded,
_TransientErrorHandled, _run_stream_attempt, StreamError, and max_transient are
updated accordingly.
🪄 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: 62ef187e-40e0-4adf-9f8e-61ee224b159a

📥 Commits

Reviewing files that changed from the base of the PR and between b8250a6 and 3e25bc7.

📒 Files selected for processing (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). (1)
  • GitHub Check: Seer Code Review
🧰 Additional context used
📓 Path-based instructions (2)
autogpt_platform/backend/**/*.py

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

autogpt_platform/backend/**/*.py: Use Python 3.11 (required; managed by Poetry via pyproject.toml) for backend development
Always run 'poetry run format' (Black + isort) before linting in backend development
Always run 'poetry run lint' (ruff) after formatting in backend development

Refer to @backend/CLAUDE.md for backend-specific commands, architecture, and development tasks

autogpt_platform/backend/**/*.py: Import only at the top level; no local/inner imports except for lazy imports of heavy optional dependencies like openpyxl
Use absolute imports with from backend.module import ... for cross-package imports; single-dot relative imports (from .sibling import ...) are acceptable for sibling modules within the same package; avoid double-dot relative imports (from ..parent import ...)
Do not use duck typing with hasattr(), getattr(), or isinstance() for type dispatch; use typed interfaces, unions, or protocols instead
Use Pydantic models for structured data instead of dataclasses, namedtuples, or dicts
Do not use linter suppressors; no # type: ignore, # noqa, or # pyright: ignore comments — fix the underlying type/code issue instead
Use list comprehensions instead of manual loop-and-append patterns
Use early return guard clauses to avoid deep nesting
Use %s for deferred interpolation in debug log statements; use f-strings for readability in other log levels (e.g., logger.debug("Processing %s items", count), logger.info(f"Processing {count} items"))
Sanitize error paths using os.path.basename() in error messages to avoid leaking directory structure
Avoid TOCTOU (time-of-check-time-of-use) patterns; do not use check-then-act patterns for file access and credit charging operations
Use Redis pipelines with transaction=True for atomicity on multi-step Redis operations
Use max(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 h...

Files:

  • autogpt_platform/backend/backend/copilot/sdk/service.py
autogpt_platform/**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

Format Python code with poetry run format

Files:

  • autogpt_platform/backend/backend/copilot/sdk/service.py
🧠 Learnings (16)
📓 Common learnings
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.
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: 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:41.600Z
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: 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: ntindle
Repo: Significant-Gravitas/AutoGPT PR: 12592
File: autogpt_platform/frontend/AGENTS.md:1-3
Timestamp: 2026-03-27T08:39:45.696Z
Learning: In Significant-Gravitas/AutoGPT, Claude is the primary coding agent. AGENTS.md files intentionally retain Claude-specific wording (e.g., "CLAUDE.md - Frontend", "This file provides guidance to Claude Code") even though AGENTS.md is the canonical cross-agent instruction source. Do not flag Claude-specific titles or phrasing in AGENTS.md files as issues.
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.
📚 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-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.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.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/sdk/service.py
📚 Learning: 2026-03-25T06:59:27.340Z
Learnt from: CR
Repo: Significant-Gravitas/AutoGPT PR: 0
File: autogpt_platform/backend/CLAUDE.md:0-0
Timestamp: 2026-03-25T06:59:27.340Z
Learning: Applies to autogpt_platform/backend/backend/api/**/*.py : Follow SSE (Server-Sent Events) protocol: use `data:` lines for frontend-parsed events (must match Zod schema), use `: comment` lines for heartbeats/status

Applied to files:

  • autogpt_platform/backend/backend/copilot/sdk/service.py
📚 Learning: 2026-02-27T15:59:00.370Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12213
File: autogpt_platform/frontend/src/app/api/openapi.json:9983-9995
Timestamp: 2026-02-27T15:59:00.370Z
Learning: Repo: Significant-Gravitas/AutoGPT PR: 12213 — OpenAPI/codegen
Learning: Ensuring a field is required in generated TS types needs two sides: (1) no default value on the Pydantic field, and (2) the OpenAPI model's "required" array must list it. For MCPToolInfo, making input_schema required in OpenAPI and removing Field(default_factory=dict) in the backend prevents optional typing drift.

Applied to files:

  • autogpt_platform/backend/backend/copilot/sdk/service.py
📚 Learning: 2026-02-27T15:59:00.370Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12213
File: autogpt_platform/frontend/src/app/api/openapi.json:9983-9995
Timestamp: 2026-02-27T15:59:00.370Z
Learning: Repo: Significant-Gravitas/AutoGPT PR: 12213 — Backend/frontend OpenAPI codegen
Learning: For MCP schema models, required OpenAPI fields must have no defaults in Pydantic. Specifically, MCPToolInfo.input_schema must be required (no Field(default_factory=dict)) so openapi.json emits it in "required", ensuring generated TS types treat input_schema as non-optional.

Applied to files:

  • autogpt_platform/backend/backend/copilot/sdk/service.py
📚 Learning: 2026-03-04T23:58:18.476Z
Learnt from: Pwuts
Repo: Significant-Gravitas/AutoGPT PR: 12284
File: autogpt_platform/frontend/src/app/api/openapi.json:11897-11900
Timestamp: 2026-03-04T23:58:18.476Z
Learning: Repo: Significant-Gravitas/AutoGPT — PR `#12284`
Backend/frontend OpenAPI codegen convention: In backend/api/features/store/model.py, the StoreSubmission and StoreSubmissionAdminView models define submitted_at: datetime | None, changes_summary: str | None, and instructions: str | None with no default. This is intentional to produce “required but nullable” fields in OpenAPI (properties appear in required[] and use anyOf [type, null]). This matches Prisma’s submittedAt DateTime? and changesSummary String?. Do not flag this as a required/nullable mismatch.

Applied to files:

  • autogpt_platform/backend/backend/copilot/sdk/service.py
📚 Learning: 2026-03-27T08:39:45.696Z
Learnt from: ntindle
Repo: Significant-Gravitas/AutoGPT PR: 12592
File: autogpt_platform/frontend/AGENTS.md:1-3
Timestamp: 2026-03-27T08:39:45.696Z
Learning: In Significant-Gravitas/AutoGPT, Claude is the primary coding agent. AGENTS.md files intentionally retain Claude-specific wording (e.g., "CLAUDE.md - Frontend", "This file provides guidance to Claude Code") even though AGENTS.md is the canonical cross-agent instruction source. Do not flag Claude-specific titles or phrasing in AGENTS.md files as issues.

Applied to files:

  • autogpt_platform/backend/backend/copilot/sdk/service.py
📚 Learning: 2026-03-12T14:42:40.552Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12385
File: autogpt_platform/backend/backend/copilot/rate_limit.py:141-170
Timestamp: 2026-03-12T14:42:40.552Z
Learning: In Significant-Gravitas/AutoGPT, `check_rate_limit` in `autogpt_platform/backend/backend/copilot/rate_limit.py` is intentionally a "pre-turn soft check" (not a hard atomic reservation). Because LLM token counts are unknown before generation completes, a strict check-and-reserve is impractical. The TOCTOU race (two concurrent turns both passing the pre-check and both committing via `record_token_usage`) is an accepted trade-off. If stricter enforcement is ever needed, the approach is a Lua script doing GET+INCRBY atomically in Redis.

Applied to files:

  • autogpt_platform/backend/backend/copilot/sdk/service.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.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.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/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.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.py

Comment thread autogpt_platform/backend/backend/copilot/sdk/service.py Outdated
Comment thread autogpt_platform/backend/backend/copilot/sdk/service.py Outdated
Comment thread autogpt_platform/backend/backend/copilot/sdk/service.py
@majdyz

majdyz commented Apr 1, 2026

Copy link
Copy Markdown
Contributor Author

/review

@autogpt-pr-reviewer

Copy link
Copy Markdown

Queued a review for PR #12636 at 3e25bc7.

Comment thread autogpt_platform/backend/backend/copilot/sdk/service.py Outdated
@majdyz
majdyz force-pushed the fix/copilot-p0-cli-internals branch from 250094b to e7c915d Compare April 1, 2026 15:05
@github-actions github-actions Bot removed the conflicts Automatically applied to PRs with merge conflicts label Apr 1, 2026
@github-actions

github-actions Bot commented Apr 1, 2026

Copy link
Copy Markdown
Contributor

Conflicts have been resolved! 🎉 A maintainer will review the pull request shortly.

Comment thread autogpt_platform/backend/backend/copilot/sdk/service.py Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ Duplicate comments (1)
autogpt_platform/backend/backend/copilot/sdk/service.py (1)

2112-2132: ⚠️ Potential issue | 🟡 Minor

TranscriptBuilder state not rolled back on transient retry.

Per past review discussion, state.transcript_builder._entries is independent from session.messages. While session.messages is correctly rolled back to pre_attempt_msg_count, the transcript builder retains mutations from the failed attempt. This can cause duplicate user entries in the uploaded transcript on retry.

The author acknowledged this and indicated it will be addressed in a follow-up commit with snapshot/restore logic. Flagging here for tracking.

🤖 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.py` around lines 2112 -
2132, The transient-retry path rolls back session.messages but not the separate
transcript state, causing duplicate entries; before starting the attempt capture
a snapshot of state.transcript_builder._entries (or call a new
TranscriptBuilder.snapshot() method) and on error/rollback (inside the transient
retry branch where session.messages = session.messages[:pre_attempt_msg_count]
is done and before continuing) restore state.transcript_builder._entries from
that snapshot (or call TranscriptBuilder.restore(snapshot)); add
snapshot/restore helpers on the TranscriptBuilder class if needed and ensure
restoration happens prior to resetting state.adapter/state.usage and continuing
the retry.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@autogpt_platform/backend/backend/copilot/sdk/service.py`:
- Around line 2112-2132: The transient-retry path rolls back session.messages
but not the separate transcript state, causing duplicate entries; before
starting the attempt capture a snapshot of state.transcript_builder._entries (or
call a new TranscriptBuilder.snapshot() method) and on error/rollback (inside
the transient retry branch where session.messages =
session.messages[:pre_attempt_msg_count] is done and before continuing) restore
state.transcript_builder._entries from that snapshot (or call
TranscriptBuilder.restore(snapshot)); add snapshot/restore helpers on the
TranscriptBuilder class if needed and ensure restoration happens prior to
resetting state.adapter/state.usage and continuing the retry.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: aa2e26a6-1f6c-4c76-a059-9e7bba1d929a

📥 Commits

Reviewing files that changed from the base of the PR and between 3e25bc7 and e7c915d.

📒 Files selected for processing (2)
  • autogpt_platform/backend/backend/copilot/config.py
  • autogpt_platform/backend/backend/copilot/sdk/service.py
🚧 Files skipped from review as they are similar to previous changes (1)
  • autogpt_platform/backend/backend/copilot/config.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: Check PR Status
  • 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)
🧰 Additional context used
📓 Path-based instructions (2)
autogpt_platform/backend/**/*.py

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

autogpt_platform/backend/**/*.py: Use Python 3.11 (required; managed by Poetry via pyproject.toml) for backend development
Always run 'poetry run format' (Black + isort) before linting in backend development
Always run 'poetry run lint' (ruff) after formatting in backend development

Refer to @backend/CLAUDE.md for backend-specific commands, architecture, and development tasks

autogpt_platform/backend/**/*.py: Import only at the top level; no local/inner imports except for lazy imports of heavy optional dependencies like openpyxl
Use absolute imports with from backend.module import ... for cross-package imports; single-dot relative imports (from .sibling import ...) are acceptable for sibling modules within the same package; avoid double-dot relative imports (from ..parent import ...)
Do not use duck typing with hasattr(), getattr(), or isinstance() for type dispatch; use typed interfaces, unions, or protocols instead
Use Pydantic models for structured data instead of dataclasses, namedtuples, or dicts
Do not use linter suppressors; no # type: ignore, # noqa, or # pyright: ignore comments — fix the underlying type/code issue instead
Use list comprehensions instead of manual loop-and-append patterns
Use early return guard clauses to avoid deep nesting
Use %s for deferred interpolation in debug log statements; use f-strings for readability in other log levels (e.g., logger.debug("Processing %s items", count), logger.info(f"Processing {count} items"))
Sanitize error paths using os.path.basename() in error messages to avoid leaking directory structure
Avoid TOCTOU (time-of-check-time-of-use) patterns; do not use check-then-act patterns for file access and credit charging operations
Use Redis pipelines with transaction=True for atomicity on multi-step Redis operations
Use max(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 h...

Files:

  • autogpt_platform/backend/backend/copilot/sdk/service.py
autogpt_platform/**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

Format Python code with poetry run format

Files:

  • autogpt_platform/backend/backend/copilot/sdk/service.py
🧠 Learnings (29)
📓 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: 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.
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: ntindle
Repo: Significant-Gravitas/AutoGPT PR: 12592
File: autogpt_platform/frontend/AGENTS.md:1-3
Timestamp: 2026-03-27T08:39:45.696Z
Learning: In Significant-Gravitas/AutoGPT, Claude is the primary coding agent. AGENTS.md files intentionally retain Claude-specific wording (e.g., "CLAUDE.md - Frontend", "This file provides guidance to Claude Code") even though AGENTS.md is the canonical cross-agent instruction source. Do not flag Claude-specific titles or phrasing in AGENTS.md files as issues.
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12636
File: autogpt_platform/backend/backend/copilot/sdk/service.py:0-0
Timestamp: 2026-04-01T14:53:59.242Z
Learning: In Significant-Gravitas/AutoGPT (autogpt_platform), `claude_agent_max_transient_retries` (default=3) in `ChatConfig` counts **total attempts including the initial one**, not the number of extra retries. With the pre-incremented `transient_retries >= max_transient` guard in `service.py`, a value of 3 yields 3 total stream attempts (initial + 2 retries with exponential backoff: 1s, 2s). Do NOT flag this as an off-by-one — the `>=` check is intentional.
📚 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-01T14:53:59.242Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12636
File: autogpt_platform/backend/backend/copilot/sdk/service.py:0-0
Timestamp: 2026-04-01T14:53:59.242Z
Learning: In Significant-Gravitas/AutoGPT (autogpt_platform), `claude_agent_max_transient_retries` (default=3) in `ChatConfig` counts **total attempts including the initial one**, not the number of extra retries. With the pre-incremented `transient_retries >= max_transient` guard in `service.py`, a value of 3 yields 3 total stream attempts (initial + 2 retries with exponential backoff: 1s, 2s). Do NOT flag this as an off-by-one — the `>=` check is intentional.

Applied to files:

  • autogpt_platform/backend/backend/copilot/sdk/service.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.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/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.py
📚 Learning: 2026-03-25T06:59:27.340Z
Learnt from: CR
Repo: Significant-Gravitas/AutoGPT PR: 0
File: autogpt_platform/backend/CLAUDE.md:0-0
Timestamp: 2026-03-25T06:59:27.340Z
Learning: Applies to autogpt_platform/backend/backend/api/**/*.py : Follow SSE (Server-Sent Events) protocol: use `data:` lines for frontend-parsed events (must match Zod schema), use `: comment` lines for heartbeats/status

Applied to files:

  • autogpt_platform/backend/backend/copilot/sdk/service.py
📚 Learning: 2026-02-27T15:59:00.370Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12213
File: autogpt_platform/frontend/src/app/api/openapi.json:9983-9995
Timestamp: 2026-02-27T15:59:00.370Z
Learning: Repo: Significant-Gravitas/AutoGPT PR: 12213 — OpenAPI/codegen
Learning: Ensuring a field is required in generated TS types needs two sides: (1) no default value on the Pydantic field, and (2) the OpenAPI model's "required" array must list it. For MCPToolInfo, making input_schema required in OpenAPI and removing Field(default_factory=dict) in the backend prevents optional typing drift.

Applied to files:

  • autogpt_platform/backend/backend/copilot/sdk/service.py
📚 Learning: 2026-02-27T15:59:00.370Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12213
File: autogpt_platform/frontend/src/app/api/openapi.json:9983-9995
Timestamp: 2026-02-27T15:59:00.370Z
Learning: Repo: Significant-Gravitas/AutoGPT PR: 12213 — Backend/frontend OpenAPI codegen
Learning: For MCP schema models, required OpenAPI fields must have no defaults in Pydantic. Specifically, MCPToolInfo.input_schema must be required (no Field(default_factory=dict)) so openapi.json emits it in "required", ensuring generated TS types treat input_schema as non-optional.

Applied to files:

  • autogpt_platform/backend/backend/copilot/sdk/service.py
📚 Learning: 2026-03-04T23:58:18.476Z
Learnt from: Pwuts
Repo: Significant-Gravitas/AutoGPT PR: 12284
File: autogpt_platform/frontend/src/app/api/openapi.json:11897-11900
Timestamp: 2026-03-04T23:58:18.476Z
Learning: Repo: Significant-Gravitas/AutoGPT — PR `#12284`
Backend/frontend OpenAPI codegen convention: In backend/api/features/store/model.py, the StoreSubmission and StoreSubmissionAdminView models define submitted_at: datetime | None, changes_summary: str | None, and instructions: str | None with no default. This is intentional to produce “required but nullable” fields in OpenAPI (properties appear in required[] and use anyOf [type, null]). This matches Prisma’s submittedAt DateTime? and changesSummary String?. Do not flag this as a required/nullable mismatch.

Applied to files:

  • autogpt_platform/backend/backend/copilot/sdk/service.py
📚 Learning: 2026-03-27T08:39:45.696Z
Learnt from: ntindle
Repo: Significant-Gravitas/AutoGPT PR: 12592
File: autogpt_platform/frontend/AGENTS.md:1-3
Timestamp: 2026-03-27T08:39:45.696Z
Learning: In Significant-Gravitas/AutoGPT, Claude is the primary coding agent. AGENTS.md files intentionally retain Claude-specific wording (e.g., "CLAUDE.md - Frontend", "This file provides guidance to Claude Code") even though AGENTS.md is the canonical cross-agent instruction source. Do not flag Claude-specific titles or phrasing in AGENTS.md files as issues.

Applied to files:

  • autogpt_platform/backend/backend/copilot/sdk/service.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: When reviewing code under autogpt_platform/backend/backend/copilot/tools/, the `AgentInfo.graph` field (in agent_search.py / models.py) uses `Graph | None` (the typed `backend.data.graph.Graph` Pydantic model), NOT `dict[str, Any]`. The enrichment function `_enrich_agents_with_graph` calls `graph_db().get_graph(graph_id, version=None, user_id=user_id)` directly rather than going through `get_agent_as_json()` / `graph_to_json()`. This was updated in PR `#12622` (commit 22d05bc).

Applied to files:

  • autogpt_platform/backend/backend/copilot/sdk/service.py
📚 Learning: 2026-03-09T10:50:43.907Z
Learnt from: Bentlybro
Repo: Significant-Gravitas/AutoGPT PR: 0
File: :0-0
Timestamp: 2026-03-09T10:50:43.907Z
Learning: Repo: Significant-Gravitas/AutoGPT — File: autogpt_platform/backend/backend/blocks/llm.py
For xAI Grok models accessed via OpenRouter, the API returns `null` for `max_completion_tokens`. The convention in this codebase is to use the model's context window size as the `max_output_tokens` value in ModelMetadata. For example, Grok 3 uses 131072 (128k) and Grok 4 uses 262144 (256k). Do not flag these as incorrect max output token values.

Applied to files:

  • autogpt_platform/backend/backend/copilot/sdk/service.py
📚 Learning: 2026-03-16T17:00:02.827Z
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).

Applied to files:

  • autogpt_platform/backend/backend/copilot/sdk/service.py
📚 Learning: 2026-03-17T06:18:51.570Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12445
File: autogpt_platform/frontend/src/app/(platform)/copilot/components/ChatContainer/ChatContainer.tsx:55-67
Timestamp: 2026-03-17T06:18:51.570Z
Learning: In `autogpt_platform/frontend/src/app/(platform)/copilot/components/ChatContainer/ChatContainer.tsx`, an explicit `isBusy` guard on the retry handler (`handleRetry`) is not needed. Once `onSend` is invoked, the chat status immediately transitions to "submitted", which causes the `ErrorCard` (containing the retry button) to unmount before a second click can register, making double-send impossible by design.

Applied to files:

  • autogpt_platform/backend/backend/copilot/sdk/service.py
📚 Learning: 2026-03-10T08:39:22.025Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12356
File: autogpt_platform/backend/backend/copilot/constants.py:9-12
Timestamp: 2026-03-10T08:39:22.025Z
Learning: In Significant-Gravitas/AutoGPT PR `#12356`, the `COPILOT_SYNTHETIC_ID_PREFIX = "copilot-"` check in `create_auto_approval_record` (human_review.py) is intentional and safe. The `graph_exec_id` passed to this function comes from server-side `PendingHumanReview` DB records (not from user input); the API only accepts `node_exec_id` from users. Synthetic `copilot-*` IDs are only ever created server-side in `run_block.py`. The prefix skip avoids a DB lookup for a `AgentGraphExecution` record that legitimately does not exist for CoPilot sessions, while `user_id` scoping is enforced at the auth layer and on the resulting auto-approval record.

Applied to files:

  • autogpt_platform/backend/backend/copilot/sdk/service.py
📚 Learning: 2026-03-12T14:42:40.552Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12385
File: autogpt_platform/backend/backend/copilot/rate_limit.py:141-170
Timestamp: 2026-03-12T14:42:40.552Z
Learning: In Significant-Gravitas/AutoGPT, `check_rate_limit` in `autogpt_platform/backend/backend/copilot/rate_limit.py` is intentionally a "pre-turn soft check" (not a hard atomic reservation). Because LLM token counts are unknown before generation completes, a strict check-and-reserve is impractical. The TOCTOU race (two concurrent turns both passing the pre-check and both committing via `record_token_usage`) is an accepted trade-off. If stricter enforcement is ever needed, the approach is a Lua script doing GET+INCRBY atomically in Redis.

Applied to files:

  • autogpt_platform/backend/backend/copilot/sdk/service.py
📚 Learning: 2026-03-26T07:00:03.405Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12574
File: autogpt_platform/backend/backend/copilot/sdk/transcript.py:980-990
Timestamp: 2026-03-26T07:00:03.405Z
Learning: In `autogpt_platform/backend/backend/copilot/sdk/transcript.py`, `_rechain_tail` intentionally rewrites `parentUuid` for **all** tail entries (not just the first), because a single assistant turn can span multiple consecutive JSONL entries sharing the same `message.id` (e.g., a thinking entry + a tool_use entry). Their original `parentUuid` values may reference entries that were absorbed into the compressed prefix, so sequential rechaining of the entire tail is required to maintain a valid parent→child graph. The test `test_chains_multiple_tail_entries` validates this: the second tail entry's `parentUuid` is rewritten from its original value to the uuid of the first tail entry.

Applied to files:

  • autogpt_platform/backend/backend/copilot/sdk/service.py
📚 Learning: 2026-03-30T11:49:37.770Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12604
File: autogpt_platform/backend/backend/copilot/sdk/security_hooks.py:165-171
Timestamp: 2026-03-30T11:49:37.770Z
Learning: In `autogpt_platform/backend/backend/copilot/sdk/security_hooks.py`, the `web_search_count` and `total_tool_call_count` circuit-breaker counters in `create_security_hooks` are intentionally per-turn (closure-local), not per-session. Hooks are recreated per stream invocation in `service.py`, so counters reset each turn. This is an accepted v1 design: it caps a single runaway turn (incident d2f7cba3: 179 WebSearch calls, $20.66). True per-session persistence via Redis is deferred to a later iteration. Do not flag these as a per-session vs. per-turn mismatch bug.

Applied to files:

  • autogpt_platform/backend/backend/copilot/sdk/service.py
📚 Learning: 2026-03-31T15:37:19.733Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12623
File: autogpt_platform/backend/backend/copilot/transcript_builder.py:30-34
Timestamp: 2026-03-31T15:37:19.733Z
Learning: In `autogpt_platform/backend/backend/copilot/transcript_builder.py` (and its re-export shim at `sdk/transcript_builder.py`), `TranscriptEntry.parentUuid` is typed `str | None` and `model_dump_json(exclude_none=True)` is used intentionally. A missing `parentUuid` in the serialized JSONL is how the CLI identifies the root entry. Do not flag `parentUuid=None` / omitted `parentUuid` as a bug — this is correct, pre-existing behavior and must not be changed to `""`.

Applied to files:

  • autogpt_platform/backend/backend/copilot/sdk/service.py
📚 Learning: 2026-03-24T02:23:33.877Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12526
File: autogpt_platform/frontend/src/app/(platform)/copilot/components/RateLimitResetDialog/RateLimitResetDialog.tsx:0-0
Timestamp: 2026-03-24T02:23:33.877Z
Learning: When handling errors in `onError` callbacks for generated Orval hooks in the Copilot platform UI (autogpt_platform/frontend), the project convention is to explicitly check for `ApiError` and read `error.response?.detail` first, falling back to `error.message` and then a generic string. While the custom Orval mutator (`autogpt_platform/frontend/src/app/api/mutators/custom-mutator.ts`) already maps `responseData?.detail` into `ApiError.message`, the explicit `error.response?.detail` extraction is still used for consistency with other hooks like `useCronSchedulerDialog.ts` and `useRunGraph.ts`. This pattern is established in `autogpt_platform/frontend/src/app/(platform)/copilot/hooks/useResetRateLimit.ts` (commit 7962185cc, PR `#12526`).

Applied to files:

  • autogpt_platform/backend/backend/copilot/sdk/service.py
📚 Learning: 2026-03-24T21:25:15.983Z
Learnt from: ntindle
Repo: Significant-Gravitas/AutoGPT PR: 12536
File: autogpt_platform/frontend/src/app/api/openapi.json:5770-5790
Timestamp: 2026-03-24T21:25:15.983Z
Learning: Repo: Significant-Gravitas/AutoGPT — PR `#12536`
File: autogpt_platform/frontend/src/app/api/openapi.json
Learning: The OpenAPI spec file is auto-generated; per established convention, endpoints generally declare only 200/201, 401, and 422 responses. Do not suggest adding explicit 403/404 response entries for single operations unless planning a repo-wide spec update. Prefer clarifying such behaviors in endpoint descriptions/docstrings instead of altering response maps.

Applied to files:

  • autogpt_platform/backend/backend/copilot/sdk/service.py
📚 Learning: 2026-03-10T08:38:36.655Z
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:36.655Z
Learning: In the AutoGPT CoPilot HITL (Human-In-The-Loop) flow (`autogpt_platform/backend/backend/copilot/tools/run_block.py`), the review card presented to users sets `editable: false`, meaning reviewers cannot modify the input payload. Therefore, credentials resolved before `is_block_exec_need_review()` remain valid and do not need to be recomputed after the review step — the original `input_data` is unchanged through the review lifecycle.

Applied to files:

  • autogpt_platform/backend/backend/copilot/sdk/service.py
📚 Learning: 2026-04-01T04:17:35.195Z
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:35.195Z
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.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/sdk/service.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.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.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.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.py
🔇 Additional comments (7)
autogpt_platform/backend/backend/copilot/sdk/service.py (7)

568-582: LGTM — provider-aware fallback model resolution.

The function correctly mirrors _resolve_sdk_model() to apply the same dot→hyphen normalization for direct Anthropic API usage, ensuring the fallback model ID is valid across auth modes.


1911-1919: LGTM — P0 guardrails properly wired to SDK.

The three guardrail fields are correctly passed to ClaudeAgentOptions:

  • fallback_model: enables SDK auto-retry on 529 overloaded errors
  • max_turns: caps agentic tool-use loops to prevent runaway execution
  • max_budget_usd: enforces per-query spend ceiling

As noted in past review comments, the SDK compatibility test (sdk_compat_test.py) should be updated to include these three fields in fields_we_use to catch future SDK breaking changes.


2001-2017: LGTM — transient retry helper with correct semantics.

The _can_retry_transient() helper correctly implements the "3 total attempts" semantics documented in learnings. The pre-incremented >= max_transient guard is intentional: with max_transient_retries=3, this yields 3 total stream attempts (initial + 2 retries with 1s, 2s backoff).

The events_yielded > 0 check correctly prevents retrying after partial output has been sent to the frontend, avoiding duplicate/inconsistent content.


2031-2035: LGTM — transient retry counter correctly reset per context-level attempt.

Resetting transient_retries = 0 at the top of each context-level attempt ensures that each attempt (original → compacted → no-transcript) gets its full transient retry budget, rather than sharing a single budget across all compaction stages.


2157-2172: LGTM — enhanced logging for transient error detection.

Adding is_transient to the exception logging provides valuable diagnostic information for debugging intermittent API failures.


2186-2214: LGTM — transient exception retry path with correct fallback.

The generic exception handler correctly:

  1. Attempts transient retries with exponential backoff before context-level compaction
  2. Falls through to context error handling only after transient retries are exhausted
  3. Surfaces non-context, non-transient errors immediately

The _can_retry_transient() helper deduplicates the logic previously repeated between this block and the _HandledStreamError handler.


1856-1867: Environment variable names are correct. The four env vars (CLAUDE_CODE_DISABLE_CLAUDE_MDS, CLAUDE_CODE_SKIP_PROMPT_HISTORY, CLAUDE_CODE_DISABLE_AUTO_MEMORY, CLAUDE_CODE_DISABLE_NONESSENTIAL_TRAFFIC) are all recognized by the Claude Code CLI. Note that CLAUDE_CODE_SKIP_PROMPT_HISTORY has a side effect (regression in v2.1.77+) where it skips all session transcripts, not just prompt history. Additionally, CLAUDE_CODE_DISABLE_NONESSENTIAL_TRAFFIC may disable gated features. Ensure these side effects are acceptable for the multi-tenant isolation model.

…x test isolation

Add two integration tests to TestStreamChatCompletionRetryIntegration that
exercise the two previously-uncovered transient retry code paths:
- _HandledStreamError(code="transient_api_error") path: AssistantMessage with
  error="rate_limit" triggers backoff, StreamStatus emitted, second attempt succeeds
- Generic Exception path: ECONNRESET raised from receive_response triggers backoff,
  StreamStatus emitted, second attempt succeeds

Also fix test isolation in _make_sdk_patches:
- Mock get_user_tier (was hitting real DB → exponential backoff hung tests locally)
- Add missing config fields: claude_agent_max_transient_retries=1,
  claude_agent_max_turns, claude_agent_max_budget_usd, claude_agent_fallback_model
@majdyz

majdyz commented Apr 9, 2026

Copy link
Copy Markdown
Contributor Author

Fixed in afe7380a3:

Blocker 1 — Missing integration test: _HandledStreamError transient retry path
Added test_handled_stream_error_transient_retries_then_succeeds: mocks _run_stream_attempt returning AssistantMessage(error='rate_limit') on first call, then a success on second. Asserts: (a) 2 SDK client calls made, (b) no StreamError yielded, (c) StreamStatus('Connection interrupted, retrying in Xs…') emitted.

Blocker 2 — Missing integration test: generic Exception transient retry path
Added test_generic_exception_transient_retry_then_succeeds: mocks receive_response raising Exception('ECONNRESET: connection reset by peer') on first call, success on second. Same assertions as above.

Also fixed test isolation in _make_sdk_patches: added get_user_tier to mocks (was hitting real DB → exponential backoff hung tests locally without Docker), and added missing config fields (claude_agent_max_transient_retries=1, max_turns, max_budget_usd, fallback_model). All 151 tests pass in 4.1s.

@majdyz

majdyz commented Apr 9, 2026

Copy link
Copy Markdown
Contributor Author

Summary of all changes since last bot review (2026-04-08T12:35)

Addresses all items flagged in the most recent autogpt-pr-reviewer-in-dev CHANGES_REQUESTED (round 5).

Blockers — both fixed in afe7380a3:

  1. Integration test: _HandledStreamError transient retry pathTestHandledStreamErrorTransientRetry.test_retry_succeeds_after_one_transient_handled_error and retry_scenarios_test.TestRealRetryLoop.test_handled_stream_error_transient_retries_then_succeeds mock _run_stream_attempt to fail with transient_api_error then succeed; assert 2 calls, StreamStatus emitted, no StreamError.

  2. Integration test: generic Exception transient retry pathTestGenericExceptionTransientRetry.test_econnreset_triggers_retry_and_succeeds and retry_scenarios_test.TestRealRetryLoop.test_generic_exception_transient_retry_then_succeeds mock ECONNRESET then success; same assertions.

Should Fix — all addressed in a5560bc4a5:

  1. Transcript builder private attribute access — Added snapshot()/restore() public API to TranscriptBuilder. Retry loop now calls these methods instead of accessing _entries/_last_uuid directly. Covered by TestTranscriptBuilderSnapshotRestore.

  2. Backoff jitter_compute_transient_backoff applies full-jitter (0.5–1.0 × base) to prevent thundering herd on concurrent 429/529s. test_backoff_capped_at_30s updated to use range assertions since jitter makes exact values non-deterministic.

  3. _last_reset_attempt infinite-loop guard untested — Added TestLastResetAttemptGuard.test_transient_retry_preserves_counter and test_counter_resets_on_new_attempt.

  4. _on_stderr fallback detection untested — Extracted _is_fallback_stderr() as module-level pure function; _on_stderr closure delegates to it. Covered by TestIsFallbackStderr (4 tests: true positive, case-insensitive, false positive, empty).

  5. Tautological TestEventsYieldedExclusions — Fixed in 47a869a32: tests now import production _EPHEMERAL_EVENT_TYPES constant rather than reconstructing the tuple locally.

Test count: 715 passing, 3 xfailed (all SDK unit tests). CI: 35/35 checks green.

… no file available

When no previous transcript file exists (storage failure or compaction drop),
the SDK path fell back to lossy plain-text injection via
_format_conversation_context which truncated tool results at 500 chars and
discarded tool_use/tool_result structural linkage (IDs, names, inputs).

Add _session_messages_to_transcript() which converts session.messages to a
proper JSONL transcript with full tool_use and tool_result content blocks.
Wire it in before SDK options are built so the Claude CLI receives complete
tool call history via --resume instead of degraded text context.

Also add 7 unit tests for _session_messages_to_transcript covering: empty
input, user/assistant messages, tool_use block generation, tool_result block
generation, full content (no truncation), parent UUID chain, and malformed
tool call argument handling.
@github-actions github-actions Bot removed the size/l label Apr 9, 2026
…ion modes

In Mode 2 (direct Anthropic) and Mode 3 (OpenRouter), the Claude CLI
subprocess was inheriting CLAUDE_CODE_OAUTH_TOKEN from the container
environment. The CLI prefers OAuth auth over ANTHROPIC_AUTH_TOKEN,
causing authentication failures when the OAuth token is expired or
belongs to a different user.

Fix: explicitly set CLAUDE_CODE_OAUTH_TOKEN="" and
CLAUDE_CODE_REFRESH_TOKEN="" in the env dict for Modes 2 and 3 so
the merged subprocess env has these cleared, forcing the CLI to use
the configured API key / auth token instead.

Add two new tests verifying the tokens are cleared in both modes.
majdyz added a commit that referenced this pull request Apr 9, 2026
@majdyz

majdyz commented Apr 9, 2026

Copy link
Copy Markdown
Contributor Author

🧪 E2E Test Report — PR #12636

E2E Test Report: PR #12636 — P0 guardrails, transient retry, security, transcript reconstruction

Date: 2026-04-09
Branch: fix/copilot-p0-cli-internals
Worktree: /Users/majdyz/Code/AutoGPT15

Environment

  • Docker: AutoGPT15 worktree build (fresh --no-cache)
  • Auth: Claude subscription (OAuth from keychain, freshly extracted)
  • Feature flags: Default (SDK mode via LaunchDarkly)
  • All platform services running

Test Results

Scenario 1: Basic SDK conversation — PASS ✅

Steps:

  1. Created new chat session via POST /api/chat/sessions
  2. Sent message: "Hello! What tools do you have available?"
  3. Streamed response

Actual: Full streaming response received with proper SSE events (start → text-start → text-delta → text-end → finish)
Logs: Using SDK service (mode=default), subscription mode CLI version 2.1.63


Scenario 2: Tool calls preserved across turns (SDK) — PASS ✅

Steps:

  1. T1: "Search for latest AutoGPT news" → run_block (PerplexityBlock) invoked
  2. T2: "Which company recently launched AutoGPT as an automotive search tool?"

Expected: T2 correctly answers from T1 tool result context
Actual: "Based on the research I just ran: OLX Group launched AutoGPT..."

Transcript logs:

  • T1: Uploaded 4.9KB transcript (3 entries)
  • T2: Downloaded 4.9KB, loaded 3 entries, resume=True — tool call context preserved ✅
  • T3: Downloaded 9.4KB, loaded 10 entries, resume=True — context still intact ✅

Scenario 3: No-transcript reconstruction (JSONL rebuild) — PASS ✅

Steps:

  1. Created new session, T1: "Search for Python tutorials" → WebSearch invoked
  2. Manually deleted transcript file from local workspace storage
  3. T2: "What was the first result from the search you did?"

Expected: _session_messages_to_transcript reconstructs JSONL from DB messages, passes to Claude CLI via --resume
Actual: Log confirms: "Reconstructed transcript from 4 session messages for --resume (no previous transcript file)"
Response: "The first result was: The Python Tutorial — Python 3.14.4 documentation"

Key verification: cache_read=23937 (system prompt cached), cache_create=1959 (new context), uncached=3 — reconstruction provided proper tool_use/tool_result structural context without lossy truncation ✅


Scenario 4: Baseline path tool calls — PASS ✅

Steps:

  1. Forced baseline via FORCE_FLAG_COPILOT_SDK=false and valid OpenRouter API key
  2. T1: "Search for Python fastest web framework in 2026"
  3. T2: "Which framework did you find was fastest?"

Expected: Baseline service invoked, tool calls work, context preserved
Actual: Log: "Using baseline service (mode=default)", 3 tool calls in T1
T2 response: "Based on my research..." — context preserved across turns ✅

Note: Required working OpenRouter key (OPEN_ROUTER_API_KEY from root .env). The previous CHAT_API_KEY in the worktree .env was expired.


Scenario 5: Compaction handling — PASS ✅

Steps:

  1. Sent 6 turns with 3-4 tool calls each (WebSearch), accumulating ~205k tokens
  2. T7: "Based on all research, what's the single most important trend?"
  3. T8: "What was the first topic we researched? What framework did you recommend?"

Expected: CLI auto-compacts when context nears 200k, context preserved post-compaction
Actual:

  • T7 token usage: cache_read=0, cache_create=127296 — compaction triggered (CLI created fresh compaction summary)
  • T7 response correctly synthesizes all prior research topics ✅
  • T8 response: "The first topic we researched was Python Web Frameworks Comparison 2026" — full context recall post-compaction ✅

Transcript grew to 399KB / 66 entries before compaction. After compaction, new transcript was 127k tokens (compaction summary). Session continued seamlessly ✅


Bug Found and Fixed

build_sdk_env — OAuth token conflict in non-subscription modes

Severity: High (breaks OpenRouter mode when container has OAuth tokens set)

Root cause: build_sdk_env Mode 3 (OpenRouter) and Mode 2 (Direct Anthropic) did not clear CLAUDE_CODE_OAUTH_TOKEN and CLAUDE_CODE_REFRESH_TOKEN. The Claude CLI subprocess inherited these from the container environment and preferred OAuth auth over the configured ANTHROPIC_AUTH_TOKEN, causing 401 "User not found" errors.

Discovery: Initial test run with CHAT_USE_CLAUDE_CODE_SUBSCRIPTION=false + OpenRouter key — CLI tried OAuth, failed. Verified by checking subprocess env: OAuth tokens present, ANTHROPIC_BASE_URL from build_sdk_env absent.

Fix: Added explicit clearing in both modes:

# Mode 2 (Direct Anthropic)
env = {
    "CLAUDE_CODE_OAUTH_TOKEN": "",
    "CLAUDE_CODE_REFRESH_TOKEN": "",
}

# Mode 3 (OpenRouter)
env = {
    "ANTHROPIC_BASE_URL": base,
    "ANTHROPIC_AUTH_TOKEN": config.api_key or "",
    "ANTHROPIC_API_KEY": "",
    "CLAUDE_CODE_OAUTH_TOKEN": "",   # NEW
    "CLAUDE_CODE_REFRESH_TOKEN": "", # NEW
}

Tests added: test_openrouter_clears_oauth_tokens, test_direct_anthropic_clears_oauth_tokens
Commit: 025ff4deec


Simplification Analysis

  1. _session_messages_to_transcript (~55 lines) — Clean, well-documented. The function size is appropriate for the logic it encodes (3 message roles × multiple field mappings).

  2. Reconstruction elif block (~45 lines in stream_response) — Could be extracted to _rebuild_transcript_from_session() helper, but the 6 shared variables make clean extraction non-trivial. Acceptable as-is.

  3. _format_conversation_context still used — Not dead code. Used for (a) gap-filling when transcript covers only a prefix, (b) final fallback when both transcript and reconstruction fail. Correct architecture.

  4. transcript_covers_prefix = False repeated — 4 occurrences in fallback branches. Acceptable verbosity for explicitness.

Summary

Scenario Result
Basic SDK conversation ✅ PASS
Tool calls preserved across turns ✅ PASS
No-transcript JSONL reconstruction ✅ PASS
Baseline path tool calls ✅ PASS
Compaction (205k tokens, 8 turns) ✅ PASS

Screenshots

01-login.png
02-copilot-ui.png
03-copilot-ready.png
04-copilot-main.png

…RY env var

The variable does not exist in the Claude Code CLI — confirmed absent from
official docs and source. Setting it was a no-op, leaving prompt history
persisting in multi-tenant deployments.
@majdyz
majdyz enabled auto-merge April 9, 2026 13:46
@majdyz
majdyz disabled auto-merge April 9, 2026 14:10
@majdyz
majdyz merged commit d113687 into dev Apr 9, 2026
38 of 39 checks passed
@majdyz
majdyz deleted the fix/copilot-p0-cli-internals branch April 9, 2026 14:10
@github-project-automation github-project-automation Bot moved this from 🚧 Needs work to ✅ Done in AutoGPT development kanban Apr 9, 2026
majdyz added a commit that referenced this pull request Apr 11, 2026
…12636)

### Why

The copilot's Claude Code CLI integration had several production
reliability gaps reported from live deployments:

- **No transient retry**: 429 rate-limit errors, 5xx server errors, and
ECONNRESET connection resets surfaced immediately as failures — there
was no retry mechanism.
- **Subagent permission errors**: CLI subprocesses wrote temp files to
`/tmp/claude-0/` which was inaccessible inside E2B sandboxes, causing
subagent spawning to report "agent completed" without actually running.
- **Missing security hardening in non-OpenRouter modes**: Security env
vars (`CLAUDE_CODE_DISABLE_CLAUDE_MDS`,
`CLAUDE_CODE_SKIP_PROMPT_HISTORY`, `CLAUDE_CODE_DISABLE_AUTO_MEMORY`,
`CLAUDE_CODE_DISABLE_NONESSENTIAL_TRAFFIC`) were only applied in the
OpenRouter path, leaving subscription and direct Anthropic modes
unprotected in multi-tenant deployment.
- **No resource guardrails**: No per-query budget cap, turn limit, or
fallback model meant a single runaway query could burn unlimited
tokens/spend.
- **Lossy transcript reconstruction**: When no transcript file was
available (storage failure or compaction drop), the old code injected a
truncated plain-text summary that cut tool results at 500 chars and
dropped `tool_use`/`tool_result` structural linkage, causing the LLM to
lose conversation context.

### What

- **SDK guardrails** (`config.py`, `sdk/service.py`): Added
`fallback_model` (auto-failover on 529 overloaded), `max_turns=1000`
(runaway prevention), `max_budget_usd=100.0` (per-query cost cap). All
configurable via env-backed `ChatConfig` fields.
- **Transient retry** (`sdk/service.py`, `constants.py`): Exponential
backoff (1s, 2s, 4s) for 429/5xx/ECONNRESET errors, retried only when
`events_yielded == 0` to avoid breaking partial streams.
`_TRANSIENT_ERROR_PATTERNS` extended with status-code-specific patterns
to avoid false positives.
- **Workspace isolation** (`sdk/env.py`): `CLAUDE_CODE_TMPDIR` now set
in all auth modes so CLI subprocesses write to the per-session workspace
directory rather than `/tmp/`.
- **Security hardening** (`sdk/env.py`): Security env vars applied
uniformly across all three auth modes (subscription, direct Anthropic,
OpenRouter) via restructured `build_sdk_env()`.
- **Transcript reconstruction** (`sdk/service.py`):
`_session_messages_to_transcript()` converts `ChatMessage.tool_calls`
and `ChatMessage.tool_call_id` to proper `tool_use`/`tool_result` JSONL
blocks for `--resume`, restoring full structural fidelity.
- **Model normalization refactor** (`sdk/service.py`):
`_resolve_fallback_model()` and `_normalize_model_name()` extracted to
share prefix-stripping and dot→hyphen conversion logic between primary
and fallback model resolution.

### How it works

**Transient retry**: `_can_retry_transient()` checks the retry budget
and returns the next backoff delay (or `None` when exhausted). Retries
are gated on `events_yielded == 0` — if any events were already streamed
to the client, we cannot retry without breaking the SSE stream
mid-response. After all retries are exhausted, `FRIENDLY_TRANSIENT_MSG`
is surfaced to the user.

**Transcript reconstruction**: When `--resume` has no on-disk session
file, `_session_messages_to_transcript()` builds a JSONL transcript from
`session.messages`, emitting `tool_use` blocks for assistant tool calls
and `tool_result` blocks (with matching IDs) for their results. This
gives Claude CLI the same structural fidelity as an on-disk session —
preserving tool call/result pairing that the old plain-text injection
lost.

**`build_sdk_env()` restructure**: The three auth modes now share a
common "epilogue" block that applies workspace isolation and security
hardening env vars regardless of which mode is active, eliminating the
previous pattern of repeating `if sdk_cwd: env["CLAUDE_CODE_TMPDIR"] =
sdk_cwd` in each branch.

### Checklist 📋

#### For code changes:
- [x] I have clearly listed my changes in the PR description
- [x] I have made a test plan
- [x] I have tested my changes according to the test plan:
- [x] 729 unit tests passing: `env_test.py`, `p0_guardrails_test.py`,
`retry_scenarios_test.py` (incl. integration tests for both transient
retry paths), `service_test.py`, `sdk_compat_test.py`,
`response_adapter_test.py`
- [x] E2E tested: live copilot session (API + UI), multi-turn, security
env vars verified in all 3 auth modes, guardrail defaults confirmed
- [x] `_session_messages_to_transcript()`: 7 unit tests covering empty
input, tool_use blocks, tool_result blocks, no truncation (10K chars
preserved), parent UUID chain, malformed argument handling
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

platform/backend AutoGPT Platform - Back end size/xl

Projects

Status: ✅ Done

Development

Successfully merging this pull request may close these issues.

1 participant