fix(copilot): prioritize block discovery over MCP and sanitize HTML errors - #12394
Merged
Conversation
…rrors - Add 'Check blocks first' section to MCP guide, directing the agent to search find_block before attempting MCP for any service not in the known servers list. Explicitly prohibit guessing MCP server URLs. - Sanitize HTTP error responses in run_mcp_tool to detect HTML error pages (e.g. raw 404 pages from non-MCP endpoints) and return a clean one-liner instead of dumping the full HTML body. Resolves SECRT-2116 --- Co-authored-by: Zamil Majdy (@majdyz) <majdyz@gmail.com>
Otto-AGPT
requested review from
Pwuts and
Swiftyos
and removed request for
a team
March 13, 2026 04:34
github-project-automation
Bot
moved this to 🆕 Needs initial review
in AutoGPT development kanban
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📜 Recent 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). (6)
🧰 Additional context used📓 Path-based instructions (5)autogpt_platform/backend/**/*.py📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
autogpt_platform/backend/**/*.{py,txt}📄 CodeRabbit inference engine (autogpt_platform/backend/CLAUDE.md)
Files:
autogpt_platform/backend/backend/**/*.py📄 CodeRabbit inference engine (autogpt_platform/backend/CLAUDE.md)
Files:
autogpt_platform/**/*.py📄 CodeRabbit inference engine (AGENTS.md)
Files:
autogpt_platform/backend/**/*test*.py📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧠 Learnings (8)📓 Common learnings📚 Learning: 2026-02-27T10:45:55.700ZApplied to files:
📚 Learning: 2026-02-27T15:59:00.370ZApplied to files:
📚 Learning: 2026-02-27T15:59:00.370ZApplied to files:
📚 Learning: 2026-02-26T17:02:22.448ZApplied to files:
📚 Learning: 2026-03-04T08:04:35.881ZApplied to files:
📚 Learning: 2026-03-04T12:19:39.243ZApplied to files:
📚 Learning: 2026-03-05T15:42:08.207ZApplied to files:
🔇 Additional comments (1)
WalkthroughReplaces registry lookup guidance with an API search example, adds explicit pre-checks (use Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Reinforce in the shared supplement (appended to the Langfuse system prompt) that the agent should always search find_block before considering MCP for any integration request.
Contributor
🔍 PR Overlap DetectionThis check compares your PR against all other open PRs targeting the same branch to detect potential merge conflicts early. 🟢 Low Risk — File Overlap OnlyThese PRs touch the same files but different sections (click to expand)
Summary: 0 conflict(s), 0 medium risk, 1 low risk (out of 1 PRs with file overlap) Auto-generated on push. Ignores: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
autogpt_platform/backend/backend/copilot/sdk/mcp_tool_guide.md (1)
31-33: Clarify fallback behavior whenfind_blockfails (not just no-results).The rule is clear for “no matching blocks,” but it doesn’t explicitly say what to do on
find_blockerrors. Adding that guard will keep tool choice deterministic.Proposed wording tweak
Only use `run_mcp_tool` when: - The service is in the known hosted MCP servers list above, OR - You searched `find_block` first and found no matching blocks +- If `find_block` returns an error, do not use `run_mcp_tool` yet; resolve the search issue first.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@autogpt_platform/backend/backend/copilot/sdk/mcp_tool_guide.md` around lines 31 - 33, The guidance is missing the fallback for find_block failures; update the text around run_mcp_tool and find_block to state that run_mcp_tool should be used when the service is in the known hosted MCP servers list OR when find_block either returns no matching blocks or fails with an error (treat errors the same as no-results to keep tool selection deterministic); reference the symbols run_mcp_tool and find_block and make the wording explicit that find_block errors trigger the same fallback to run_mcp_tool.autogpt_platform/backend/backend/copilot/tools/run_mcp_tool.py (1)
193-206: Avoid logging raw HTTP error bodies after sanitizing user output.Line 193 still logs
str(e)directly, which can include full HTML pages. Consider logging status/host plus an HTML flag instead.Proposed log-sanitization refactor
- logger.warning("MCP HTTP error for %s: %s", server_host(server_url), e) # Sanitize error: strip HTML bodies (e.g. raw 404 pages) to avoid # dumping entire HTML documents into the agent/user-facing message. error_body = str(e) - if _looks_like_html(error_body): + is_html_error = _looks_like_html(error_body) + logger.warning( + "MCP HTTP error for %s (status=%s, html_error=%s)", + server_host(server_url), + e.status_code, + is_html_error, + ) + if is_html_error: error_msg = ( f"MCP server at {server_host(server_url)} returned HTTP " f"{e.status_code}. " "This URL does not appear to host an MCP server." )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@autogpt_platform/backend/backend/copilot/tools/run_mcp_tool.py` around lines 193 - 206, The logger currently prints the raw exception object (str(e)) which can contain full HTML bodies; change the logging in run_mcp_tool.py so it only logs the host, HTTP status (e.status_code or e.code), and whether the body looks like HTML instead of the raw body string: capture error_body = str(e) only for inspection, compute is_html = _looks_like_html(error_body), then call logger.warning with a sanitized message like "MCP HTTP error for %s: status=%s html=%s" using server_host(server_url), status, is_html (and if needed include e.__class__.__name__), and keep the existing branch that builds error_msg using error_body[:200] only for user-facing output but never log the unsanitized full body.
🤖 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/tools/run_mcp_tool.py`:
- Around line 194-209: Add a unit test that mocks an HTTPClientError containing
an HTML-like body (e.g., "<!doctype html>...") to exercise the _looks_like_html
branch in run_mcp_tool.py: call the function that catches HTTPClientError (the
routine that returns ErrorResponse) with a mocked exception having .status_code
and string() returning the HTML body, then assert the returned
ErrorResponse.message contains "This URL does not appear to host an MCP server."
and preserves the session_id; reference symbols to locate code:
_looks_like_html, ErrorResponse, server_host/server_url, and HTTPClientError.
---
Nitpick comments:
In `@autogpt_platform/backend/backend/copilot/sdk/mcp_tool_guide.md`:
- Around line 31-33: The guidance is missing the fallback for find_block
failures; update the text around run_mcp_tool and find_block to state that
run_mcp_tool should be used when the service is in the known hosted MCP servers
list OR when find_block either returns no matching blocks or fails with an error
(treat errors the same as no-results to keep tool selection deterministic);
reference the symbols run_mcp_tool and find_block and make the wording explicit
that find_block errors trigger the same fallback to run_mcp_tool.
In `@autogpt_platform/backend/backend/copilot/tools/run_mcp_tool.py`:
- Around line 193-206: The logger currently prints the raw exception object
(str(e)) which can contain full HTML bodies; change the logging in
run_mcp_tool.py so it only logs the host, HTTP status (e.status_code or e.code),
and whether the body looks like HTML instead of the raw body string: capture
error_body = str(e) only for inspection, compute is_html =
_looks_like_html(error_body), then call logger.warning with a sanitized message
like "MCP HTTP error for %s: status=%s html=%s" using server_host(server_url),
status, is_html (and if needed include e.__class__.__name__), and keep the
existing branch that builds error_msg using error_body[:200] only for
user-facing output but never log the unsanitized full body.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: cc7a4405-c3e5-495c-ad73-3035a97efc1a
📒 Files selected for processing (2)
autogpt_platform/backend/backend/copilot/sdk/mcp_tool_guide.mdautogpt_platform/backend/backend/copilot/tools/run_mcp_tool.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). (6)
- GitHub Check: types
- GitHub Check: Seer Code Review
- GitHub Check: test (3.12)
- GitHub Check: test (3.11)
- GitHub Check: test (3.13)
- GitHub Check: Check PR Status
🧰 Additional context used
📓 Path-based instructions (4)
autogpt_platform/backend/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
autogpt_platform/backend/**/*.py: Use Python 3.11 (required; managed by Poetry via pyproject.toml) for backend development
Always run 'poetry run format' (Black + isort) before linting in backend development
Always run 'poetry run lint' (ruff) after formatting in backend development
Files:
autogpt_platform/backend/backend/copilot/tools/run_mcp_tool.py
autogpt_platform/backend/**/*.{py,txt}
📄 CodeRabbit inference engine (autogpt_platform/backend/CLAUDE.md)
Use
poetry runprefix for all Python commands, including testing, linting, formatting, and migrations
Files:
autogpt_platform/backend/backend/copilot/tools/run_mcp_tool.py
autogpt_platform/backend/backend/**/*.py
📄 CodeRabbit inference engine (autogpt_platform/backend/CLAUDE.md)
Use Prisma ORM for database operations in PostgreSQL with pgvector for embeddings
Files:
autogpt_platform/backend/backend/copilot/tools/run_mcp_tool.py
autogpt_platform/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Format Python code with
poetry run format
Files:
autogpt_platform/backend/backend/copilot/tools/run_mcp_tool.py
🧠 Learnings (8)
📓 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:13.707Z
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: 12213
File: autogpt_platform/frontend/src/app/api/openapi.json:10030-10037
Timestamp: 2026-03-01T07:59:02.311Z
Learning: Repo: Significant-Gravitas/AutoGPT PR: 12213 — For MCP manual token storage, backend model autogpt_platform/backend/backend/api/features/mcp/routes.py defines MCPStoreTokenRequest.token as Pydantic SecretStr with a min length constraint, which generates OpenAPI schema metadata (format: "password", writeOnly: true, minLength: 1) in autogpt_platform/frontend/src/app/api/openapi.json. Prefer SecretStr (with length constraints) for sensitive request fields so generated TS clients and docs treat them as secrets.
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.
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.
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12213
File: autogpt_platform/frontend/src/app/(platform)/copilot/tools/RunMCPTool/helpers.tsx:23-24
Timestamp: 2026-02-27T10:45:55.700Z
Learning: As of PR `#12213`, MCP tool response types (MCPToolsDiscoveredResponse, MCPToolOutputResponse) are defined in openapi.json and frontend code in autogpt_platform/frontend/src/app/(platform)/copilot/tools/RunMCPTool/helpers.tsx uses the generated types from `@/app/api/__generated__/`. Other tools like RunBlock still use inline TypeScript interfaces (e.g., BlockDetailsResponse) for SSE stream payloads that are not included in openapi.json schemas. The pattern is tool-specific: use generated types when available in openapi.json, use inline types only when the payload schema is truly SSE-stream-only and not exposed via OpenAPI.
📚 Learning: 2026-02-27T10:45:55.700Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12213
File: autogpt_platform/frontend/src/app/(platform)/copilot/tools/RunMCPTool/helpers.tsx:23-24
Timestamp: 2026-02-27T10:45:55.700Z
Learning: As of PR `#12213`, MCP tool response types (MCPToolsDiscoveredResponse, MCPToolOutputResponse) are defined in openapi.json and frontend code in autogpt_platform/frontend/src/app/(platform)/copilot/tools/RunMCPTool/helpers.tsx uses the generated types from `@/app/api/__generated__/`. Other tools like RunBlock still use inline TypeScript interfaces (e.g., BlockDetailsResponse) for SSE stream payloads that are not included in openapi.json schemas. The pattern is tool-specific: use generated types when available in openapi.json, use inline types only when the payload schema is truly SSE-stream-only and not exposed via OpenAPI.
Applied to files:
autogpt_platform/backend/backend/copilot/tools/run_mcp_tool.pyautogpt_platform/backend/backend/copilot/sdk/mcp_tool_guide.md
📚 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/tools/run_mcp_tool.pyautogpt_platform/backend/backend/copilot/sdk/mcp_tool_guide.md
📚 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/tools/run_mcp_tool.pyautogpt_platform/backend/backend/copilot/sdk/mcp_tool_guide.md
📚 Learning: 2026-02-26T17:02:22.448Z
Learnt from: Pwuts
Repo: Significant-Gravitas/AutoGPT PR: 12211
File: .pre-commit-config.yaml:160-179
Timestamp: 2026-02-26T17:02:22.448Z
Learning: Keep the pre-commit hook pattern broad for autogpt_platform/backend to ensure OpenAPI schema changes are captured. Do not narrow to backend/api/ alone, since the generated schema depends on Pydantic models across multiple directories (backend/data/, backend/blocks/, backend/copilot/, backend/integrations/, backend/util/). Narrowing could miss schema changes and cause frontend type desynchronization.
Applied to files:
autogpt_platform/backend/backend/copilot/tools/run_mcp_tool.pyautogpt_platform/backend/backend/copilot/sdk/mcp_tool_guide.md
📚 Learning: 2026-03-04T08:04:35.881Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12273
File: autogpt_platform/backend/backend/copilot/tools/workspace_files.py:216-220
Timestamp: 2026-03-04T08:04:35.881Z
Learning: In the AutoGPT Copilot backend, ensure that SVG images are not treated as vision image types by excluding 'image/svg+xml' from INLINEABLE_MIME_TYPES and MULTIMODAL_TYPES in tool_adapter.py; the Claude API supports PNG, JPEG, GIF, and WebP for vision. SVGs (XML text) should be handled via the text path instead, not the vision path.
Applied to files:
autogpt_platform/backend/backend/copilot/tools/run_mcp_tool.py
📚 Learning: 2026-03-04T12:19:39.243Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12279
File: autogpt_platform/backend/backend/copilot/tools/base.py:184-188
Timestamp: 2026-03-04T12:19:39.243Z
Learning: In autogpt_platform/backend/backend/copilot/tools/, ensure that anonymous users always pass user_id=None to tool execution methods. The anon_ prefix (e.g., anon_123) is used only for PostHog/analytics distinct_id and must not be used as an actual user_id. Use a simple truthiness check on user_id (e.g., if user_id: ... else: ... or a dedicated is_authenticated flag) to distinguish anonymous from authenticated users, and review all tool execution call sites within this directory to prevent accidentally forwarding an anon_ user_id to tools.
Applied to files:
autogpt_platform/backend/backend/copilot/tools/run_mcp_tool.py
📚 Learning: 2026-03-05T15:42:08.207Z
Learnt from: ntindle
Repo: Significant-Gravitas/AutoGPT PR: 12297
File: .claude/skills/backend-check/SKILL.md:14-16
Timestamp: 2026-03-05T15:42:08.207Z
Learning: In Python files under autogpt_platform/backend (recursively), rely on poetry run format to perform formatting (Black + isort) and linting (ruff). Do not run poetry run lint as a separate step after poetry run format, since format already includes linting checks.
Applied to files:
autogpt_platform/backend/backend/copilot/tools/run_mcp_tool.py
🔇 Additional comments (1)
autogpt_platform/backend/backend/copilot/tools/run_mcp_tool.py (1)
37-40: HTML detection helper is a good addition.This is a lightweight guard and fits the error-sanitization path well.
…try URL Address review feedback: - Replace _looks_like_html heuristic with status code check (404/405/406 → 'not an MCP server'). Cleaner and more reliable. - Put raw HTTP details in ErrorResponse.error field so the frontend renders them in the de-emphasised pre block, not the main message. - Replace vague registry homepage link with the exact working API endpoint: GET /v0/servers?q=<search_term>, and instruct the agent to use remotes[].url from results instead of guessing paths.
Block-first guidance already lives in mcp_tool_guide.md (loaded via get_mcp_guide). No need to duplicate it in the system prompt supplement.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
autogpt_platform/backend/backend/copilot/tools/run_mcp_tool.py (1)
193-199: Prefer URL-level wording over host-level wording in not-found errors.Using only
hostcan be inaccurate when the domain is valid but the MCP path is wrong. Useserver_urlin the message for precision.Proposed message tweak
- error_msg = ( - f"No MCP server found at {host} (HTTP {e.status_code}). " + error_msg = ( + f"No MCP server found at {server_url} (HTTP {e.status_code}). " "This URL does not appear to host an MCP server." )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@autogpt_platform/backend/backend/copilot/tools/run_mcp_tool.py` around lines 193 - 199, The error message uses host which can be misleading when the MCP path is wrong; update the construction of error_msg in run_mcp_tool.py (the if block that checks e.status_code in _NOT_MCP_STATUS_CODES and the else branch building error_msg) to reference server_url (the full URL variable) instead of host so the message accurately reflects the URL that was queried; keep the existing wording but replace host with server_url in those f-strings.autogpt_platform/backend/backend/copilot/sdk/mcp_tool_guide.md (1)
24-26: Add a language to the fenced registry example.The fenced block at Line 24 is missing a language tag (MD040).
Proposed doc-only fix
-``` +```http GET https://registry.modelcontextprotocol.io/v0/servers?q=<search_term></details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@autogpt_platform/backend/backend/copilot/sdk/mcp_tool_guide.mdaround lines
24 - 26, The fenced code block containing the GET request example is missing a
language tag; update the fenced block that contains "GET
https://registry.modelcontextprotocol.io/v0/servers?q=<search_term>" to use a
language tag (e.g., add ```http at the opening fence) so the snippet is
correctly tagged and MD040 is resolved.</details> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In@autogpt_platform/backend/backend/copilot/sdk/mcp_tool_guide.md:
- Around line 24-26: The fenced code block containing the GET request example is
missing a language tag; update the fenced block that contains "GET
https://registry.modelcontextprotocol.io/v0/servers?q=<search_term>" to use a
language tag (e.g., add ```http at the opening fence) so the snippet is
correctly tagged and MD040 is resolved.In
@autogpt_platform/backend/backend/copilot/tools/run_mcp_tool.py:
- Around line 193-199: The error message uses host which can be misleading when
the MCP path is wrong; update the construction of error_msg in run_mcp_tool.py
(the if block that checks e.status_code in _NOT_MCP_STATUS_CODES and the else
branch building error_msg) to reference server_url (the full URL variable)
instead of host so the message accurately reflects the URL that was queried;
keep the existing wording but replace host with server_url in those f-strings.</details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: Organization UI **Review profile**: CHILL **Plan**: Pro **Run ID**: `924a17b2-1787-4ac1-8063-ceb1fc1fc829` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between d614ae94e3fa46425bfb4082d6e075312ace0595 and c88ac7d11c9b36550b53f3960f0d85d8b2334811. </details> <details> <summary>📒 Files selected for processing (2)</summary> * `autogpt_platform/backend/backend/copilot/sdk/mcp_tool_guide.md` * `autogpt_platform/backend/backend/copilot/tools/run_mcp_tool.py` </details> </details> <details> <summary>📜 Review details</summary> <details> <summary>⏰ 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). (5)</summary> * GitHub Check: types * GitHub Check: test (3.12) * GitHub Check: test (3.11) * GitHub Check: test (3.13) * GitHub Check: Check PR Status </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>📓 Path-based instructions (4)</summary> <details> <summary>autogpt_platform/backend/**/*.py</summary> **📄 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 Files: - `autogpt_platform/backend/backend/copilot/tools/run_mcp_tool.py` </details> <details> <summary>autogpt_platform/backend/**/*.{py,txt}</summary> **📄 CodeRabbit inference engine (autogpt_platform/backend/CLAUDE.md)** > Use `poetry run` prefix for all Python commands, including testing, linting, formatting, and migrations Files: - `autogpt_platform/backend/backend/copilot/tools/run_mcp_tool.py` </details> <details> <summary>autogpt_platform/backend/backend/**/*.py</summary> **📄 CodeRabbit inference engine (autogpt_platform/backend/CLAUDE.md)** > Use Prisma ORM for database operations in PostgreSQL with pgvector for embeddings Files: - `autogpt_platform/backend/backend/copilot/tools/run_mcp_tool.py` </details> <details> <summary>autogpt_platform/**/*.py</summary> **📄 CodeRabbit inference engine (AGENTS.md)** > Format Python code with `poetry run format` Files: - `autogpt_platform/backend/backend/copilot/tools/run_mcp_tool.py` </details> </details><details> <summary>🧠 Learnings (10)</summary> <details> <summary>📓 Common learnings</summary> ``` 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. ``` ``` 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. ``` ``` Learnt from: majdyz Repo: Significant-Gravitas/AutoGPT PR: 12213 File: autogpt_platform/frontend/src/app/api/openapi.json:10030-10037 Timestamp: 2026-03-01T07:59:02.311Z Learning: Repo: Significant-Gravitas/AutoGPT PR: 12213 — For MCP manual token storage, backend model autogpt_platform/backend/backend/api/features/mcp/routes.py defines MCPStoreTokenRequest.token as Pydantic SecretStr with a min length constraint, which generates OpenAPI schema metadata (format: "password", writeOnly: true, minLength: 1) in autogpt_platform/frontend/src/app/api/openapi.json. Prefer SecretStr (with length constraints) for sensitive request fields so generated TS clients and docs treat them as secrets. ``` ``` Learnt from: majdyz Repo: Significant-Gravitas/AutoGPT PR: 12356 File: autogpt_platform/backend/backend/copilot/constants.py:9-12 Timestamp: 2026-03-10T08:39:13.707Z 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: 12213 File: autogpt_platform/frontend/src/app/(platform)/copilot/tools/RunMCPTool/helpers.tsx:23-24 Timestamp: 2026-02-27T10:45:55.700Z Learning: As of PR `#12213`, MCP tool response types (MCPToolsDiscoveredResponse, MCPToolOutputResponse) are defined in openapi.json and frontend code in autogpt_platform/frontend/src/app/(platform)/copilot/tools/RunMCPTool/helpers.tsx uses the generated types from `@/app/api/__generated__/`. Other tools like RunBlock still use inline TypeScript interfaces (e.g., BlockDetailsResponse) for SSE stream payloads that are not included in openapi.json schemas. The pattern is tool-specific: use generated types when available in openapi.json, use inline types only when the payload schema is truly SSE-stream-only and not exposed via OpenAPI. ``` </details> <details> <summary>📚 Learning: 2026-02-27T10:45:55.700Z</summary> ``` Learnt from: majdyz Repo: Significant-Gravitas/AutoGPT PR: 12213 File: autogpt_platform/frontend/src/app/(platform)/copilot/tools/RunMCPTool/helpers.tsx:23-24 Timestamp: 2026-02-27T10:45:55.700Z Learning: As of PR `#12213`, MCP tool response types (MCPToolsDiscoveredResponse, MCPToolOutputResponse) are defined in openapi.json and frontend code in autogpt_platform/frontend/src/app/(platform)/copilot/tools/RunMCPTool/helpers.tsx uses the generated types from `@/app/api/__generated__/`. Other tools like RunBlock still use inline TypeScript interfaces (e.g., BlockDetailsResponse) for SSE stream payloads that are not included in openapi.json schemas. The pattern is tool-specific: use generated types when available in openapi.json, use inline types only when the payload schema is truly SSE-stream-only and not exposed via OpenAPI. ``` **Applied to files:** - `autogpt_platform/backend/backend/copilot/tools/run_mcp_tool.py` - `autogpt_platform/backend/backend/copilot/sdk/mcp_tool_guide.md` </details> <details> <summary>📚 Learning: 2026-02-27T15:59:00.370Z</summary> ``` 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/tools/run_mcp_tool.py` - `autogpt_platform/backend/backend/copilot/sdk/mcp_tool_guide.md` </details> <details> <summary>📚 Learning: 2026-02-27T15:59:00.370Z</summary> ``` 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/tools/run_mcp_tool.py` - `autogpt_platform/backend/backend/copilot/sdk/mcp_tool_guide.md` </details> <details> <summary>📚 Learning: 2026-03-10T08:39:13.707Z</summary> ``` Learnt from: majdyz Repo: Significant-Gravitas/AutoGPT PR: 12356 File: autogpt_platform/backend/backend/copilot/constants.py:9-12 Timestamp: 2026-03-10T08:39:13.707Z 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/tools/run_mcp_tool.py` </details> <details> <summary>📚 Learning: 2026-02-26T17:02:22.448Z</summary> ``` Learnt from: Pwuts Repo: Significant-Gravitas/AutoGPT PR: 12211 File: .pre-commit-config.yaml:160-179 Timestamp: 2026-02-26T17:02:22.448Z Learning: Keep the pre-commit hook pattern broad for autogpt_platform/backend to ensure OpenAPI schema changes are captured. Do not narrow to backend/api/ alone, since the generated schema depends on Pydantic models across multiple directories (backend/data/, backend/blocks/, backend/copilot/, backend/integrations/, backend/util/). Narrowing could miss schema changes and cause frontend type desynchronization. ``` **Applied to files:** - `autogpt_platform/backend/backend/copilot/tools/run_mcp_tool.py` - `autogpt_platform/backend/backend/copilot/sdk/mcp_tool_guide.md` </details> <details> <summary>📚 Learning: 2026-03-04T08:04:35.881Z</summary> ``` Learnt from: majdyz Repo: Significant-Gravitas/AutoGPT PR: 12273 File: autogpt_platform/backend/backend/copilot/tools/workspace_files.py:216-220 Timestamp: 2026-03-04T08:04:35.881Z Learning: In the AutoGPT Copilot backend, ensure that SVG images are not treated as vision image types by excluding 'image/svg+xml' from INLINEABLE_MIME_TYPES and MULTIMODAL_TYPES in tool_adapter.py; the Claude API supports PNG, JPEG, GIF, and WebP for vision. SVGs (XML text) should be handled via the text path instead, not the vision path. ``` **Applied to files:** - `autogpt_platform/backend/backend/copilot/tools/run_mcp_tool.py` </details> <details> <summary>📚 Learning: 2026-03-04T12:19:39.243Z</summary> ``` Learnt from: majdyz Repo: Significant-Gravitas/AutoGPT PR: 12279 File: autogpt_platform/backend/backend/copilot/tools/base.py:184-188 Timestamp: 2026-03-04T12:19:39.243Z Learning: In autogpt_platform/backend/backend/copilot/tools/, ensure that anonymous users always pass user_id=None to tool execution methods. The anon_ prefix (e.g., anon_123) is used only for PostHog/analytics distinct_id and must not be used as an actual user_id. Use a simple truthiness check on user_id (e.g., if user_id: ... else: ... or a dedicated is_authenticated flag) to distinguish anonymous from authenticated users, and review all tool execution call sites within this directory to prevent accidentally forwarding an anon_ user_id to tools. ``` **Applied to files:** - `autogpt_platform/backend/backend/copilot/tools/run_mcp_tool.py` </details> <details> <summary>📚 Learning: 2026-03-05T15:42:08.207Z</summary> ``` Learnt from: ntindle Repo: Significant-Gravitas/AutoGPT PR: 12297 File: .claude/skills/backend-check/SKILL.md:14-16 Timestamp: 2026-03-05T15:42:08.207Z Learning: In Python files under autogpt_platform/backend (recursively), rely on poetry run format to perform formatting (Black + isort) and linting (ruff). Do not run poetry run lint as a separate step after poetry run format, since format already includes linting checks. ``` **Applied to files:** - `autogpt_platform/backend/backend/copilot/tools/run_mcp_tool.py` </details> <details> <summary>📚 Learning: 2026-03-01T07:59:02.311Z</summary> ``` Learnt from: majdyz Repo: Significant-Gravitas/AutoGPT PR: 12213 File: autogpt_platform/frontend/src/app/api/openapi.json:10030-10037 Timestamp: 2026-03-01T07:59:02.311Z Learning: Repo: Significant-Gravitas/AutoGPT PR: 12213 — For MCP manual token storage, backend model autogpt_platform/backend/backend/api/features/mcp/routes.py defines MCPStoreTokenRequest.token as Pydantic SecretStr with a min length constraint, which generates OpenAPI schema metadata (format: "password", writeOnly: true, minLength: 1) in autogpt_platform/frontend/src/app/api/openapi.json. Prefer SecretStr (with length constraints) for sensitive request fields so generated TS clients and docs treat them as secrets. ``` **Applied to files:** - `autogpt_platform/backend/backend/copilot/sdk/mcp_tool_guide.md` </details> </details><details> <summary>🪛 markdownlint-cli2 (0.21.0)</summary> <details> <summary>autogpt_platform/backend/backend/copilot/sdk/mcp_tool_guide.md</summary> [warning] 24-24: Fenced code blocks should have a language specified (MD040, fenced-code-language) </details> </details> </details> <details> <summary>🔇 Additional comments (2)</summary><blockquote> <details> <summary>autogpt_platform/backend/backend/copilot/sdk/mcp_tool_guide.md (1)</summary><blockquote> `29-40`: **Strong guardrails on block-first flow and URL provenance.** This section is clear and correctly enforces `find_block` first, with explicit prohibition on guessed MCP URLs. </blockquote></details> <details> <summary>autogpt_platform/backend/backend/copilot/tools/run_mcp_tool.py (1)</summary><blockquote> `201-206`: **Good separation of primary error message vs raw HTTP detail.** Moving HTTP detail into `error` while keeping `message` concise is a solid UX and safety improvement. </blockquote></details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
- Drop _NOT_MCP_STATUS_CODES set; use single clean error message for all non-auth HTTP errors instead of guessing whether URL hosts MCP - Sanitize logger to only log host and status code, not raw body - Add test for HTML error body sanitization (404 with HTML body) - Add language tag to fenced code block in mcp_tool_guide.md
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
autogpt_platform/backend/backend/copilot/tools/test_run_mcp_tool.py (1)
616-622: Consider asserting the HTML body is preserved in theerrorfield.The test validates that HTML doesn't leak into
response.messagebut doesn't verify the raw HTML body is present inresponse.error(for the collapsible detail feature). Per the implementation inrun_mcp_tool.py:193, the error field should contain the truncated exception message (str(e)[:300]), which includes the HTML body.💡 Suggested additional assertion
# Raw detail goes in the collapsible `error` field assert response.error is not None assert "404" in response.error + # Verify raw HTML body is preserved in the collapsible error field + assert "<!doctype" in response.error.lower()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@autogpt_platform/backend/backend/copilot/tools/test_run_mcp_tool.py` around lines 616 - 622, Add an assertion in test_run_mcp_tool.py to ensure the raw HTML body appears in the collapsible detail (response.error) as implemented in run_mcp_tool.py (where the exception message is truncated via str(e)[:300]); update the test near the existing checks (after asserting response.error is not None and contains "404") to assert that "<!doctype" (or another expected HTML snippet) is present in response.error and/or that response.error contains the truncated exception string pattern emitted by the run_mcp_tool.py error handling.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@autogpt_platform/backend/backend/copilot/tools/test_run_mcp_tool.py`:
- Around line 616-622: Add an assertion in test_run_mcp_tool.py to ensure the
raw HTML body appears in the collapsible detail (response.error) as implemented
in run_mcp_tool.py (where the exception message is truncated via str(e)[:300]);
update the test near the existing checks (after asserting response.error is not
None and contains "404") to assert that "<!doctype" (or another expected HTML
snippet) is present in response.error and/or that response.error contains the
truncated exception string pattern emitted by the run_mcp_tool.py error
handling.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5dfe847c-feb1-4ee7-82f8-67cf03233f52
📒 Files selected for processing (3)
autogpt_platform/backend/backend/copilot/sdk/mcp_tool_guide.mdautogpt_platform/backend/backend/copilot/tools/run_mcp_tool.pyautogpt_platform/backend/backend/copilot/tools/test_run_mcp_tool.py
🚧 Files skipped from review as they are similar to previous changes (1)
- autogpt_platform/backend/backend/copilot/tools/run_mcp_tool.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). (6)
- GitHub Check: types
- GitHub Check: Seer Code Review
- GitHub Check: test (3.13)
- GitHub Check: test (3.11)
- GitHub Check: test (3.12)
- GitHub Check: Check PR Status
🧰 Additional context used
📓 Path-based instructions (5)
autogpt_platform/backend/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
autogpt_platform/backend/**/*.py: Use Python 3.11 (required; managed by Poetry via pyproject.toml) for backend development
Always run 'poetry run format' (Black + isort) before linting in backend development
Always run 'poetry run lint' (ruff) after formatting in backend development
Files:
autogpt_platform/backend/backend/copilot/tools/test_run_mcp_tool.py
autogpt_platform/backend/**/*.{py,txt}
📄 CodeRabbit inference engine (autogpt_platform/backend/CLAUDE.md)
Use
poetry runprefix for all Python commands, including testing, linting, formatting, and migrations
Files:
autogpt_platform/backend/backend/copilot/tools/test_run_mcp_tool.py
autogpt_platform/backend/backend/**/*.py
📄 CodeRabbit inference engine (autogpt_platform/backend/CLAUDE.md)
Use Prisma ORM for database operations in PostgreSQL with pgvector for embeddings
Files:
autogpt_platform/backend/backend/copilot/tools/test_run_mcp_tool.py
autogpt_platform/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Format Python code with
poetry run format
Files:
autogpt_platform/backend/backend/copilot/tools/test_run_mcp_tool.py
autogpt_platform/backend/**/*test*.py
📄 CodeRabbit inference engine (AGENTS.md)
Run
poetry run testfor backend testing (runs pytest with docker based postgres + prisma)
Files:
autogpt_platform/backend/backend/copilot/tools/test_run_mcp_tool.py
🧠 Learnings (8)
📓 Common learnings
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.
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.
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12213
File: autogpt_platform/frontend/src/app/api/openapi.json:10030-10037
Timestamp: 2026-03-01T07:59:02.311Z
Learning: Repo: Significant-Gravitas/AutoGPT PR: 12213 — For MCP manual token storage, backend model autogpt_platform/backend/backend/api/features/mcp/routes.py defines MCPStoreTokenRequest.token as Pydantic SecretStr with a min length constraint, which generates OpenAPI schema metadata (format: "password", writeOnly: true, minLength: 1) in autogpt_platform/frontend/src/app/api/openapi.json. Prefer SecretStr (with length constraints) for sensitive request fields so generated TS clients and docs treat them as secrets.
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12356
File: autogpt_platform/backend/backend/copilot/constants.py:9-12
Timestamp: 2026-03-10T08:39:13.707Z
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: 12213
File: autogpt_platform/frontend/src/app/(platform)/copilot/tools/RunMCPTool/helpers.tsx:23-24
Timestamp: 2026-02-27T10:45:55.700Z
Learning: As of PR `#12213`, MCP tool response types (MCPToolsDiscoveredResponse, MCPToolOutputResponse) are defined in openapi.json and frontend code in autogpt_platform/frontend/src/app/(platform)/copilot/tools/RunMCPTool/helpers.tsx uses the generated types from `@/app/api/__generated__/`. Other tools like RunBlock still use inline TypeScript interfaces (e.g., BlockDetailsResponse) for SSE stream payloads that are not included in openapi.json schemas. The pattern is tool-specific: use generated types when available in openapi.json, use inline types only when the payload schema is truly SSE-stream-only and not exposed via OpenAPI.
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.
📚 Learning: 2026-02-27T10:45:55.700Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12213
File: autogpt_platform/frontend/src/app/(platform)/copilot/tools/RunMCPTool/helpers.tsx:23-24
Timestamp: 2026-02-27T10:45:55.700Z
Learning: As of PR `#12213`, MCP tool response types (MCPToolsDiscoveredResponse, MCPToolOutputResponse) are defined in openapi.json and frontend code in autogpt_platform/frontend/src/app/(platform)/copilot/tools/RunMCPTool/helpers.tsx uses the generated types from `@/app/api/__generated__/`. Other tools like RunBlock still use inline TypeScript interfaces (e.g., BlockDetailsResponse) for SSE stream payloads that are not included in openapi.json schemas. The pattern is tool-specific: use generated types when available in openapi.json, use inline types only when the payload schema is truly SSE-stream-only and not exposed via OpenAPI.
Applied to files:
autogpt_platform/backend/backend/copilot/tools/test_run_mcp_tool.pyautogpt_platform/backend/backend/copilot/sdk/mcp_tool_guide.md
📚 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/tools/test_run_mcp_tool.pyautogpt_platform/backend/backend/copilot/sdk/mcp_tool_guide.md
📚 Learning: 2026-02-26T17:02:22.448Z
Learnt from: Pwuts
Repo: Significant-Gravitas/AutoGPT PR: 12211
File: .pre-commit-config.yaml:160-179
Timestamp: 2026-02-26T17:02:22.448Z
Learning: Keep the pre-commit hook pattern broad for autogpt_platform/backend to ensure OpenAPI schema changes are captured. Do not narrow to backend/api/ alone, since the generated schema depends on Pydantic models across multiple directories (backend/data/, backend/blocks/, backend/copilot/, backend/integrations/, backend/util/). Narrowing could miss schema changes and cause frontend type desynchronization.
Applied to files:
autogpt_platform/backend/backend/copilot/tools/test_run_mcp_tool.pyautogpt_platform/backend/backend/copilot/sdk/mcp_tool_guide.md
📚 Learning: 2026-03-04T08:04:35.881Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12273
File: autogpt_platform/backend/backend/copilot/tools/workspace_files.py:216-220
Timestamp: 2026-03-04T08:04:35.881Z
Learning: In the AutoGPT Copilot backend, ensure that SVG images are not treated as vision image types by excluding 'image/svg+xml' from INLINEABLE_MIME_TYPES and MULTIMODAL_TYPES in tool_adapter.py; the Claude API supports PNG, JPEG, GIF, and WebP for vision. SVGs (XML text) should be handled via the text path instead, not the vision path.
Applied to files:
autogpt_platform/backend/backend/copilot/tools/test_run_mcp_tool.py
📚 Learning: 2026-03-04T12:19:39.243Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12279
File: autogpt_platform/backend/backend/copilot/tools/base.py:184-188
Timestamp: 2026-03-04T12:19:39.243Z
Learning: In autogpt_platform/backend/backend/copilot/tools/, ensure that anonymous users always pass user_id=None to tool execution methods. The anon_ prefix (e.g., anon_123) is used only for PostHog/analytics distinct_id and must not be used as an actual user_id. Use a simple truthiness check on user_id (e.g., if user_id: ... else: ... or a dedicated is_authenticated flag) to distinguish anonymous from authenticated users, and review all tool execution call sites within this directory to prevent accidentally forwarding an anon_ user_id to tools.
Applied to files:
autogpt_platform/backend/backend/copilot/tools/test_run_mcp_tool.py
📚 Learning: 2026-03-05T15:42:08.207Z
Learnt from: ntindle
Repo: Significant-Gravitas/AutoGPT PR: 12297
File: .claude/skills/backend-check/SKILL.md:14-16
Timestamp: 2026-03-05T15:42:08.207Z
Learning: In Python files under autogpt_platform/backend (recursively), rely on poetry run format to perform formatting (Black + isort) and linting (ruff). Do not run poetry run lint as a separate step after poetry run format, since format already includes linting checks.
Applied to files:
autogpt_platform/backend/backend/copilot/tools/test_run_mcp_tool.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/mcp_tool_guide.md
🔇 Additional comments (1)
autogpt_platform/backend/backend/copilot/sdk/mcp_tool_guide.md (1)
23-40: LGTM!The documentation additions clearly address the issues outlined in the PR objectives:
- Registry API search replaces guesswork with an authoritative lookup
- "Check blocks first" section enforces the correct workflow
- Explicit prohibition against URL guessing prevents the original bug (e.g., guessing
https://sheets.googleapis.com/mcp)This guidance should effectively prevent CoPilot from bypassing block discovery and constructing invalid MCP URLs.
majdyz
enabled auto-merge
March 13, 2026 06:01
8 tasks
majdyz
self-requested a review
March 13, 2026 11:48
autogpt-reviewer
left a comment
autogpt-reviewer
left a comment
There was a problem hiding this comment.
PR #12394 — fix(copilot): prioritize block discovery over MCP and sanitize HTML errors
Author: Otto-AGPT | Requested by: @majdyz | Files: mcp_tool_guide.md (+18/-1), run_mcp_tool.py (+4/-2), test_run_mcp_tool.py (+43/+0)
🎯 Verdict: APPROVE
What This PR Does
When users ask CoPilot for Google Sheets integration, the agent was skipping 55+ available blocks, fabricating a fake MCP URL (sheets.googleapis.com/mcp), and dumping raw HTML 404 error pages into the conversation. This PR fixes the agent guide to enforce "check blocks first" behavior and sanitizes HTTP error responses so users see clean messages instead of raw HTML.
Specialist Findings
🛡️ Security ✅ — No vulnerabilities. The error field (truncated to 300 chars) could expose remote server internals, but this is low-risk since it's in a separate debug field, not the user-facing message. server_host() correctly strips credentials via urlparse().hostname. No new SSRF surface. Logger no longer dumps raw exception bodies — a security improvement.
🏗️ Architecture ✅ — Error restructuring (message vs error field) is consistent with existing ErrorResponse patterns across the codebase (search_docs.py, workspace_files.py, feature_requests.py). Guide-based enforcement is pragmatic given the LLM-driven tool dispatch architecture. No new coupling introduced. Suggestion: consider extracting _MAX_ERROR_DETAIL_LENGTH = 300 as a constant (nit).
⚡ Performance ✅ — No concerns. All changes are on the error path only. str(e)[:300] is O(1) in CPython, slices by code points (not bytes) so no Unicode issues. The server_host() call is now cached in a local variable — minor improvement. Guide changes are static prompt content with zero runtime cost.
🧪 Testing message, raw detail preserved in error). Missing edge cases (non-blocking): (1) no test for the 300-char truncation with long bodies, (2) no parametrized coverage of non-HTML/empty error bodies, (3) existing test_auth_error_with_existing_creds_returns_error should be updated to verify the new error field. These are should-fix for a follow-up.
📖 Quality ✅ — Guide is clear and actionable. Error messages are user-friendly. Minor nits: (1) unnecessary parentheses in message=(f"...") on line 190, (2) "MCP request" in user-facing message could be simplified to "Request" since users may not know what MCP means, (3) inline import in test could be moved to top-level for consistency.
📦 Product ✅ — Directly addresses the critical UX failure. Users now see "MCP request to sheets.googleapis.com failed with HTTP 404." instead of raw HTML dumps. The guide's "Check blocks first" section is well-positioned. Important caveat: the main CoPilot system prompt (managed externally) also needs updating to reinforce block-first behavior — the PR acknowledges this. This PR is still worth merging standalone as it makes things strictly better.
📬 Discussion ✅ — All CodeRabbit review threads resolved. majdyz addressed the actionable item (HTML test coverage) and simplified the approach. Remaining CodeRabbit nitpicks are cosmetic. No unresolved concerns. Zero independent human approvals on record — PR needs formal approval to merge. Low overlap risk with PR #12206 (different sections of same test file).
🔎 QA ErrorResponse.error as de-emphasized <pre> with text-xs opacity-80 in RunMCPTool.tsx. The error field is always visible (not behind a <details> collapsible despite test name), but this is acceptable for 300-char truncated content. No UI/UX blockers found.
Blockers
None.
Should Fix (Follow-up OK)
- System prompt update — The
mcp_tool_guide.mdchanges only take effect when the agent callsget_mcp_guide, which happens after it decides to pursue MCP. The block-first instruction needs to be in the system prompt's capability-check section to prevent entering the MCP workflow prematurely. (PR acknowledges this — track as required follow-up.) test_run_mcp_tool.py— Add edge case tests: truncation at 300 chars with long bodies, non-HTML error bodies, empty bodies. Update existing 403-with-creds test to verify newerrorfield.run_mcp_tool.py:190— Remove unnecessary parentheses:message=(f"...")→message=f"...".
Nice to Have
- Extract
_MAX_ERROR_DETAIL_LENGTH = 300as a named constant. - Consider simplifying user-facing message from "MCP request to {host}" to "Request to {host}" since users may not know what MCP is.
- Consider making the
errordetail collapsible (<details>) in the frontend for cleaner UX. - Move "Check blocks first" section above "Known hosted MCP servers" in the guide to match natural priority order.
Risk Assessment
Merge risk: LOW | Rollback: EASY
Small, well-scoped change (65 lines across 3 files). Error path only — no happy-path behavior changes. All CI green. No schema/migration changes.
@ntindle Clean fix for a critical UX bug — CoPilot no longer dumps raw HTML into conversations. No blockers; should-fix items are all follow-up quality. Recommend merge.
majdyz
approved these changes
Mar 14, 2026
github-project-automation
Bot
moved this from 🆕 Needs initial review
to 👍🏼 Mergeable
in AutoGPT development kanban
github-project-automation
Bot
moved this from 👍🏼 Mergeable
to ✅ Done
in AutoGPT development kanban
6 tasks
Bentlybro
pushed a commit
that referenced
this pull request
Apr 4, 2026
…rrors (#12394) Requested by @majdyz When a user asks for Google Sheets integration, the CoPilot agent skips block discovery entirely (despite 55+ Google Sheets blocks being available), jumps straight to MCP, guesses a fake URL (`https://sheets.googleapis.com/mcp`), and gets a raw HTML 404 error page dumped into the conversation. **Changes:** 1. **MCP guide** (`mcp_tool_guide.md`): Added "Check blocks first" section directing the agent to use `find_block` before attempting MCP for any service not in the known servers list. Explicitly prohibits guessing/constructing MCP server URLs. 2. **Error handling** (`run_mcp_tool.py`): Detects HTML error pages in HTTP responses (e.g. raw 404 pages from non-MCP endpoints) and returns a clean one-liner like "This URL does not appear to host an MCP server" instead of dumping the full HTML body. **Note:** The main CoPilot system prompt (managed externally, not in repo) should also be updated to reinforce block-first behavior in the Capability Check section. This PR covers the in-repo changes. Session reference: `9216df83-5f4a-48eb-9457-3ba2057638ae` (turn 3) Ticket: [SECRT-2116](https://linear.app/autogpt/issue/SECRT-2116) --- Co-authored-by: Zamil Majdy (@majdyz) <majdyz@gmail.com> --------- Co-authored-by: Zamil Majdy (@majdyz) <majdyz@gmail.com> Co-authored-by: Zamil Majdy <zamil.majdy@agpt.co>
Bentlybro
pushed a commit
to Bentlybro/AutoGPT
that referenced
this pull request
Apr 4, 2026
…rrors (Significant-Gravitas#12394) Requested by @majdyz When a user asks for Google Sheets integration, the CoPilot agent skips block discovery entirely (despite 55+ Google Sheets blocks being available), jumps straight to MCP, guesses a fake URL (`https://sheets.googleapis.com/mcp`), and gets a raw HTML 404 error page dumped into the conversation. **Changes:** 1. **MCP guide** (`mcp_tool_guide.md`): Added "Check blocks first" section directing the agent to use `find_block` before attempting MCP for any service not in the known servers list. Explicitly prohibits guessing/constructing MCP server URLs. 2. **Error handling** (`run_mcp_tool.py`): Detects HTML error pages in HTTP responses (e.g. raw 404 pages from non-MCP endpoints) and returns a clean one-liner like "This URL does not appear to host an MCP server" instead of dumping the full HTML body. **Note:** The main CoPilot system prompt (managed externally, not in repo) should also be updated to reinforce block-first behavior in the Capability Check section. This PR covers the in-repo changes. Session reference: `9216df83-5f4a-48eb-9457-3ba2057638ae` (turn 3) Ticket: [SECRT-2116](https://linear.app/autogpt/issue/SECRT-2116) --- Co-authored-by: Zamil Majdy (@majdyz) <majdyz@gmail.com> --------- Co-authored-by: Zamil Majdy (@majdyz) <majdyz@gmail.com> Co-authored-by: Zamil Majdy <zamil.majdy@agpt.co>
12 tasks
7 tasks
ntindle
pushed a commit
to goingforstudying-ctrl/AutoGPT
that referenced
this pull request
Jun 5, 2026
…block exists (Significant-Gravitas#13117) ### Why / What / How **Why:** [SECRT-2350](https://linear.app/agpt/issue/SECRT-2350) — Today a user has to *know* that MCP exists for a given service (e.g. MailerLite) and explicitly ask AutoPilot to connect via MCP. Non-technical users won't know this is even an option, so requests for services without a native block get rejected as "no integration exists" even when an MCP integration is available. And when MCP *is* used, the raw "MCP server / OAuth" jargon is intimidating to users who don't know the term. **What:** Prompt-only changes to the CoPilot tool-discovery surface (no code/tool/schema changes). Note that the auto-discovery groundwork and most jargon-avoidance already landed on `dev` via parallel PRs (Significant-Gravitas#12394, Significant-Gravitas#13207); against current `dev` this PR contributes three things: 1. **Discover MCP servers via web search instead of the MCP registry API.** The previous guidance queried `https://registry.modelcontextprotocol.io/v0/servers?q=...`. That's a hardcoded `/v0/` endpoint (would break on a `v1` bump) and its `q=` search didn't reliably surface real servers — e.g. searching "sentry" returns no Sentry result. We drop the registry lookup entirely: the fallback chain is now **native block → known hosted-server list → web search for the service's official MCP URL → give up**. 2. **Reframe MCP as "an integration (MCP)" in user-facing text.** The model leads with the word "integration" and discloses `(MCP)` once, in parentheses, the first time it's mentioned in a turn — then drops it. This keeps non-technical users from being scared by the term while staying transparent for users who *do* know what MCP is. Raw jargon ("MCP server", "MCP tool", "OAuth", "credentials") stays banned in user-facing text. 3. **Harden the MCP guide** so MCP discovery is explicitly mandatory (not optional) before declaring a service unsupported, with the new web-search step in the required checklist. **How:** - `backend/backend/copilot/prompting.py` — step 2 of the Tool Discovery Priority ladder now web-searches for the official MCP URL (instead of querying the registry); the hostname-verification guard applies to search-returned URLs and notes they're unvetted; adds the "(MCP)" user-facing framing note. The anti-pattern section and "Correct flow" pseudocode are updated from "registry lookup" to "web search". - `backend/backend/copilot/sdk/mcp_tool_guide.md` — replaces the registry-API section with web-search guidance; the "Check blocks first" section becomes "Check blocks first, then MCP is MANDATORY" with a known-list → web-search checklist; the "Communication style" section adopts the "the <Service> integration (MCP)" convention. **Safety note:** Web-search results are unvetted, so the prompt requires the model to verify the server hostname is vendor-owned (e.g. `mcp.sentry.dev` for Sentry) before any sign-in, and to ask the user which candidate to use when the match is ambiguous — because the user is about to hand that URL their sign-in. No new tools, no schema changes, no migration. The cacheable system prompt in `service.py` is untouched, so the prompt cache is preserved (edits land only in the per-turn shareable tool notes and the on-demand MCP guide). ### Changes 🏗️ - `prompting.py`: registry-API lookup replaced with web search for the MCP server URL; hostname-verification note broadened to unvetted search results; new "(MCP)" user-facing framing line; anti-pattern + "Correct flow" updated to reference web search. - `mcp_tool_guide.md`: registry-API section replaced with web-search guidance; "Check blocks first" hardened to "...then MCP is MANDATORY" with a 2-step (known list → web search) checklist; "Communication style" section adopts the "<Service> integration (MCP)" convention. ### Checklist 📋 #### For code changes: - [x] I have clearly listed my changes in the PR description - [x] I have made a test plan - [ ] I have tested my changes according to the test plan: - [ ] Ask AutoPilot to use a service with no native block but a known MCP server (e.g. Linear, Notion, Stripe) — confirm it connects without the user mentioning MCP. - [ ] Ask AutoPilot to use a service with no native block and not in the known list (e.g. MailerLite, Sentry) — confirm it web-searches for the official MCP URL before saying it's unsupported. - [ ] Confirm the model verifies the hostname is vendor-owned and asks the user when multiple candidate URLs are ambiguous. - [ ] Ask AutoPilot to use a service with a native block (e.g. Google Sheets) — confirm it still uses `find_block` first and doesn't unnecessarily reach for MCP. - [ ] Confirm user-facing chat reads "the X integration (MCP)" on first mention and "the X integration" / "X" thereafter; no "MCP server", "OAuth", or "credentials" leaks. - [ ] Ask for a service with neither a block nor an MCP server — confirm the model only declines *after* both `find_block` and a web search return nothing. #### For configuration changes: - [x] `.env.default` is updated or already compatible with my changes - [x] `docker-compose.yml` is updated or already compatible with my changes - [x] I have included a list of my configuration changes in the PR description (under **Changes**) --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Requested by @majdyz
When a user asks for Google Sheets integration, the CoPilot agent skips block discovery entirely (despite 55+ Google Sheets blocks being available), jumps straight to MCP, guesses a fake URL (
https://sheets.googleapis.com/mcp), and gets a raw HTML 404 error page dumped into the conversation.Changes:
MCP guide (
mcp_tool_guide.md): Added "Check blocks first" section directing the agent to usefind_blockbefore attempting MCP for any service not in the known servers list. Explicitly prohibits guessing/constructing MCP server URLs.Error handling (
run_mcp_tool.py): Detects HTML error pages in HTTP responses (e.g. raw 404 pages from non-MCP endpoints) and returns a clean one-liner like "This URL does not appear to host an MCP server" instead of dumping the full HTML body.Note: The main CoPilot system prompt (managed externally, not in repo) should also be updated to reinforce block-first behavior in the Capability Check section. This PR covers the in-repo changes.
Session reference:
9216df83-5f4a-48eb-9457-3ba2057638ae(turn 3)Ticket: SECRT-2116
Co-authored-by: Zamil Majdy (@majdyz) majdyz@gmail.com