feat(copilot): Add browse_web tool for JS-rendered page browsing - #12214
feat(copilot): Add browse_web tool for JS-rendered page browsing#12214majdyz wants to merge 5 commits into
Conversation
Adds a new `browse_web` copilot tool backed by Stagehand + Browserbase that handles JavaScript-rendered pages, SPAs, and dynamic content that the existing `web_fetch` tool cannot reach. - BrowseWebTool: navigates URLs with a real browser, extracts content via natural language instruction using Stagehand's extract() API - Ephemeral sessions per call — no external storage needed - Thread-safe signal handling for the CoPilot executor thread pool - 50K char content limit to protect LLM context window - Graceful degradation when Stagehand env vars are not configured - Registered in TOOL_REGISTRY alongside web_fetch Requires: STAGEHAND_API_KEY, STAGEHAND_PROJECT_ID, ANTHROPIC_API_KEY
github-project-automation
Bot
moved this to 🆕 Needs initial review
in AutoGPT development kanban
WalkthroughAdds a new BrowseWebTool that uses Stagehand to navigate and extract JavaScript-rendered page content, updates models and OpenAPI to include a browse_web response type, and includes unit tests and registration of the tool. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant BrowseWebTool as "BrowseWebTool"
participant StagehandAPI as "Stagehand API"
participant PageModel as "Page / Extraction Model"
Client->>BrowseWebTool: request(url, instruction)
BrowseWebTool->>BrowseWebTool: validate input & env
BrowseWebTool->>StagehandAPI: init client (lazy import)
StagehandAPI-->>BrowseWebTool: client
BrowseWebTool->>StagehandAPI: navigate to URL (with timeout)
StagehandAPI->>PageModel: render page & run extraction model
PageModel-->>StagehandAPI: extracted content
StagehandAPI-->>BrowseWebTool: content/result
BrowseWebTool->>BrowseWebTool: truncate if needed
BrowseWebTool-->>Client: BrowseWebResponse / ErrorResponse
BrowseWebTool->>StagehandAPI: close client
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🔍 PR Overlap DetectionThis check compares your PR against all other open PRs targeting the same branch to detect potential merge conflicts early. 🟡 Medium Risk — Some Line OverlapThese PRs have some overlapping changes:
🟢 Low Risk — File Overlap OnlyThese PRs touch the same files but different sections (click to expand)
Summary: 0 conflict(s), 1 medium risk, 1 low risk (out of 2 PRs with file overlap) Auto-generated on push. Ignores: |
Adds browse_web to the ResponseType enum in the OpenAPI schema to match the new BrowseWebTool response type added to the backend.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
majdyz
left a comment
majdyz
left a comment
There was a problem hiding this comment.
Inline notes on key design decisions in this PR.
| """ | ||
|
|
||
| import logging | ||
| import os |
There was a problem hiding this comment.
Why monkey-patch instead of importing from blocks/stagehand/blocks.py?
Stagehand registers OS signal handlers on __init__, which raises ValueError in non-main threads (the CoPilot executor thread pool). The same patch is applied in blocks.py but importing it from there would create a copilot/tools → blocks dependency, which is an unusual direction for this codebase. Duplicating the 5-line patch here keeps the tools module self-contained.
Matches the exact same pattern used in blocks/stagehand/blocks.py:38-48.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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/__init__.py`:
- Line 13: The eager import of BrowseWebTool causes unguarded stagehand imports
in browse_web.py to break tool initialization if stagehand is missing; update
browse_web.py to either move the stagehand imports into the
BrowseWebTool._execute method (so imports occur only at runtime) or guard the
top-level imports with try/except and make TOOL_REGISTRY register BrowseWebTool
conditionally/lazily (e.g., only add BrowseWebTool to TOOL_REGISTRY if the
stagehand import succeeds); ensure references to BrowseWebTool and TOOL_REGISTRY
remain unchanged so other tools register normally.
In `@autogpt_platform/backend/backend/copilot/tools/browse_web.py`:
- Around line 183-186: The truncation logic appends "\n\n[Content truncated]"
after slicing to _MAX_CONTENT_CHARS, which causes the final content to exceed
the 50,000-character cap; adjust the truncation so that when truncated is True
you slice content to _MAX_CONTENT_CHARS minus the length of the suffix (or
otherwise ensure len(content) <= _MAX_CONTENT_CHARS) before appending the
suffix, updating the block that sets truncated and mutates content (variables:
truncated, content, constant: _MAX_CONTENT_CHARS, and the appended suffix
string) so the returned content never exceeds the cap.
- Around line 195-200: The user-facing ErrorResponse currently includes raw
exception text (variable e) in browse_web's exception handler; change it to a
generic message (e.g., "Failed to browse URL") and an appropriate error code
("browse_failed"), log the full exception details internally using
logger.exception or logger.warning(..., exc_info=True) for debugging, and ensure
session_id and other safe fields remain, so no provider/internal exception text
is returned to users in the ErrorResponse.
- Around line 52-60: Remove the redundant context manager _thread_safe_signal
and its usage: delete the _thread_safe_signal generator function definition, and
where the code uses "with _thread_safe_signal():" (the usage wrapping the
worker/thread setup), remove the with line and dedent the inner block so the
wrapped code runs directly; rely on the existing _register_signal_handlers patch
instead of mutating signal.signal from worker threads.
- Around line 176-177: The page navigation and extraction calls use
page.goto(url) and page.extract(instruction) without Stagehand timeouts; update
both calls to pass an explicit timeoutMs value (e.g., page.goto(url,
timeoutMs=...) and page.extract(instruction, timeoutMs=...)) instead of using
asyncio.timeout(), picking the appropriate millisecond value from existing
config/constants or introduce a sensible default constant and reuse it so both
operations have bounded timeouts.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
autogpt_platform/backend/backend/copilot/tools/__init__.pyautogpt_platform/backend/backend/copilot/tools/browse_web.pyautogpt_platform/backend/backend/copilot/tools/models.pyautogpt_platform/frontend/src/app/api/openapi.json
📜 Review details
🧰 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/__init__.pyautogpt_platform/backend/backend/copilot/tools/models.pyautogpt_platform/backend/backend/copilot/tools/browse_web.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/__init__.pyautogpt_platform/backend/backend/copilot/tools/models.pyautogpt_platform/backend/backend/copilot/tools/browse_web.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/__init__.pyautogpt_platform/backend/backend/copilot/tools/models.pyautogpt_platform/backend/backend/copilot/tools/browse_web.py
autogpt_platform/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Format Python code with
poetry run format
Files:
autogpt_platform/backend/backend/copilot/tools/__init__.pyautogpt_platform/backend/backend/copilot/tools/models.pyautogpt_platform/backend/backend/copilot/tools/browse_web.py
🧠 Learnings (2)
📚 Learning: 2026-02-27T07:26:32.993Z
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-27T07:26:32.993Z
Learning: In autogpt_platform/frontend/src/app/(platform)/copilot/tools/**/helpers.tsx files, inline TypeScript interfaces for tool response types (e.g., MCPToolsDiscoveredOutput, BlockDetailsResponse) are intentional for SSE stream payloads that don't appear in openapi.json. Only ResponseType enum values are generated. This pattern should not be flagged for replacement with generated types.
Applied to files:
autogpt_platform/frontend/src/app/api/openapi.json
📚 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/__init__.pyautogpt_platform/backend/backend/copilot/tools/models.pyautogpt_platform/backend/backend/copilot/tools/browse_web.py
🧬 Code graph analysis (3)
autogpt_platform/backend/backend/copilot/tools/__init__.py (1)
autogpt_platform/backend/backend/copilot/tools/browse_web.py (1)
BrowseWebTool(65-207)
autogpt_platform/backend/backend/copilot/tools/models.py (1)
autogpt_platform/backend/backend/copilot/response_model.py (1)
ResponseType(20-44)
autogpt_platform/backend/backend/copilot/tools/browse_web.py (3)
autogpt_platform/backend/backend/copilot/model.py (1)
ChatSession(126-302)autogpt_platform/backend/backend/copilot/tools/base.py (1)
BaseTool(16-119)autogpt_platform/backend/backend/copilot/tools/models.py (3)
BrowseWebResponse(443-449)ErrorResponse(205-210)ToolResponseBase(56-61)
🔇 Additional comments (5)
autogpt_platform/backend/backend/copilot/tools/models.py (2)
44-45:ResponseType.BROWSE_WEBis correctly integrated.The new enum value is consistent with the tool naming and response model usage.
443-450:BrowseWebResponsemodel shape looks correct.The response fields (
url,content,truncated) and typed discriminator align well with browse_web output expectations.autogpt_platform/backend/backend/copilot/tools/__init__.py (1)
54-55:browse_webregistry wiring is clean.The registry entry matches the tool name and keeps discovery/execution paths consistent.
autogpt_platform/frontend/src/app/api/openapi.json (1)
11170-11170:ResponseTypeenum update is aligned with the new tool response.Adding
"browse_web"on Line 11170 keeps the frontend discriminator list in sync with the backend tool response type.autogpt_platform/backend/backend/copilot/tools/browse_web.py (1)
19-21: The current implementation is correct.stagehandis a required dependency declared inpyproject.toml, not optional. Required dependencies should not be wrapped in try-except; they should fail at import time if missing, which is the correct behavior. This pattern is intentionally used consistently across the codebase—autogpt_platform/backend/backend/blocks/stagehand/blocks.pyuses the identical approach without defensive wrapping. The monkey patching of_register_signal_handlersis already defensive via the_safe_register_signal_handlerswrapper, which correctly checks thread context before calling the original handler.
Design notes — key decisions in this PR1. Signal handler monkey-patch (browse_web.py:38-48) Stagehand calls 2. Each 3. 50K char content cap (browse_web.py:20) Stagehand's 4. No session/cookie persistence (by design) Each call creates a fresh ephemeral Browserbase session. Persistent sessions (for sites requiring login) can be added later by storing the Browserbase 5. Overlap with #12213 ( Both PRs add a new entry to |
- Lazy-import Stagehand inside _execute with ImportError guard so missing stagehand package never breaks other tools in the registry - Remove _thread_safe_signal context manager (redundant + globally mutates signal.signal from worker threads, which is harmful) - Add _patch_stagehand_once() with double-checked locking — thread-safe one-shot monkey-patch identical to the pattern in blocks/stagehand/blocks.py - Add explicit timeoutMs to page.goto() (30 s) and page.extract() (60 s) - Fix truncation: compute keep = MAX - len(suffix) to stay within cap - Catch Exception without binding `e`; log via logger.exception(); return generic message to avoid leaking raw exception text to callers
CodeRabbit review — follow-up (commit 9ae1bda)All 5 actionable items addressed: 🔴 Critical — unguarded module-level stagehand imports ( 🟠 Major — 🟠 Major — no explicit timeouts on 🟠 Major — truncation exceeded the 50 000-char cap 🟠 Major — raw exception text leaked to user |
37 tests covering: - Tool metadata (name, requires_auth, parameter schema, registry registration) - Input validation (missing/empty/non-HTTP URLs) - Env var checks (missing STAGEHAND_API_KEY, STAGEHAND_PROJECT_ID, ANTHROPIC_API_KEY) - Stagehand absent (ImportError → ErrorResponse, other tools unaffected) - Successful browse (response shape, instruction forwarding, timeout constants, session_id propagation, client.close() called) - Truncation (within limit, over limit, exact limit, empty/None extraction) - Error handling (generic message, no raw exception leakage, finally close, close() failure does not propagate) - Thread safety (_patch_stagehand_once idempotence, worker-thread no-op) All tests run without a running server or database via sys.modules injection.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
autogpt_platform/backend/backend/copilot/tools/browse_web_test.py (1)
157-175: Consider parametrizing the URL-scheme rejection cases.The
ftp://,file://, andjavascript:tests are structurally identical and can be collapsed into one parametrized test for easier maintenance.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@autogpt_platform/backend/backend/copilot/tools/browse_web_test.py` around lines 157 - 175, The three identical tests for rejected URL schemes (test_ftp_url_rejected, test_file_url_rejected, test_javascript_url_rejected) should be collapsed into a single parametrized test: create one async test (e.g., test_rejected_url_schemes) that uses pytest.mark.parametrize with a list of URLs ("ftp://example.com/file", "file:///etc/passwd", "javascript:alert(1)") and calls BrowseWebTool()._execute(user_id="u1", session=make_session(), url=url) for each case, asserting the result is an ErrorResponse and keeping the existing scheme-specific assertion (e.g., for ftp assert "http" in result.message.lower() if still needed) or otherwise asserting a generic error message; remove the three duplicate test functions.
🤖 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/browse_web.py`:
- Around line 136-141: The prefix-only URL check is too weak; update the
validation in the browse_web handler (the code that checks the variable `url`
and currently returns `ErrorResponse`) to parse the URL with
urllib.parse.urlparse, verify that parsed.scheme.lower() is either "http" or
"https", and ensure parsed.netloc is non-empty (reject if empty or missing);
replace the startswith branch with this parsed-scheme + netloc check and return
the same ErrorResponse (error="invalid_url") when validation fails so malformed
or mixed-case schemes are rejected and true HTTP/HTTPS URLs are accepted.
---
Nitpick comments:
In `@autogpt_platform/backend/backend/copilot/tools/browse_web_test.py`:
- Around line 157-175: The three identical tests for rejected URL schemes
(test_ftp_url_rejected, test_file_url_rejected, test_javascript_url_rejected)
should be collapsed into a single parametrized test: create one async test
(e.g., test_rejected_url_schemes) that uses pytest.mark.parametrize with a list
of URLs ("ftp://example.com/file", "file:///etc/passwd", "javascript:alert(1)")
and calls BrowseWebTool()._execute(user_id="u1", session=make_session(),
url=url) for each case, asserting the result is an ErrorResponse and keeping the
existing scheme-specific assertion (e.g., for ftp assert "http" in
result.message.lower() if still needed) or otherwise asserting a generic error
message; remove the three duplicate test functions.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
autogpt_platform/backend/backend/copilot/tools/browse_web.pyautogpt_platform/backend/backend/copilot/tools/browse_web_test.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Seer Code Review
- GitHub Check: types
- GitHub Check: test (3.12)
- GitHub Check: test (3.13)
- GitHub Check: test (3.11)
- GitHub Check: end-to-end tests
- GitHub Check: Check PR Status
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (6)
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/browse_web_test.pyautogpt_platform/backend/backend/copilot/tools/browse_web.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/browse_web_test.pyautogpt_platform/backend/backend/copilot/tools/browse_web.py
autogpt_platform/backend/**/*_test.py
📄 CodeRabbit inference engine (autogpt_platform/backend/CLAUDE.md)
autogpt_platform/backend/**/*_test.py: Always review snapshot changes withgit diffbefore committing when updating snapshots withpoetry run pytest --snapshot-update
Use pytest with snapshot testing for API responses in test files
Colocate test files with source files using the*_test.pynaming convention
Files:
autogpt_platform/backend/backend/copilot/tools/browse_web_test.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/browse_web_test.pyautogpt_platform/backend/backend/copilot/tools/browse_web.py
autogpt_platform/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Format Python code with
poetry run format
Files:
autogpt_platform/backend/backend/copilot/tools/browse_web_test.pyautogpt_platform/backend/backend/copilot/tools/browse_web.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/browse_web_test.py
🧠 Learnings (2)
📚 Learning: 2026-01-28T18:29:34.362Z
Learnt from: CR
Repo: Significant-Gravitas/AutoGPT PR: 0
File: autogpt_platform/frontend/src/tests/CLAUDE.md:0-0
Timestamp: 2026-01-28T18:29:34.362Z
Learning: Applies to autogpt_platform/frontend/src/tests/src/tests/**/*.spec.ts : Use E2E tests (Playwright) for flows requiring real browser APIs (clipboard, downloads) or cross-page navigation
Applied to files:
autogpt_platform/backend/backend/copilot/tools/browse_web_test.py
📚 Learning: 2026-02-26T17:02:22.448Z
Learnt from: Pwuts
Repo: Significant-Gravitas/AutoGPT PR: 12211
File: .pre-commit-config.yaml:160-179
Timestamp: 2026-02-26T17:02:22.448Z
Learning: Keep the pre-commit hook pattern broad for autogpt_platform/backend to ensure OpenAPI schema changes are captured. Do not narrow to backend/api/ alone, since the generated schema depends on Pydantic models across multiple directories (backend/data/, backend/blocks/, backend/copilot/, backend/integrations/, backend/util/). Narrowing could miss schema changes and cause frontend type desynchronization.
Applied to files:
autogpt_platform/backend/backend/copilot/tools/browse_web_test.pyautogpt_platform/backend/backend/copilot/tools/browse_web.py
🧬 Code graph analysis (2)
autogpt_platform/backend/backend/copilot/tools/browse_web_test.py (2)
autogpt_platform/backend/backend/copilot/tools/browse_web.py (2)
BrowseWebTool(68-227)_patch_stagehand_once(44-65)autogpt_platform/backend/backend/copilot/tools/models.py (1)
BrowseWebResponse(443-449)
autogpt_platform/backend/backend/copilot/tools/browse_web.py (3)
autogpt_platform/backend/backend/copilot/model.py (1)
ChatSession(126-302)autogpt_platform/backend/backend/copilot/tools/base.py (1)
BaseTool(16-119)autogpt_platform/backend/backend/copilot/tools/models.py (3)
BrowseWebResponse(443-449)ErrorResponse(205-210)ToolResponseBase(56-61)
🔇 Additional comments (4)
autogpt_platform/backend/backend/copilot/tools/browse_web_test.py (2)
250-439: Great execution-path coverage for the tool lifecycle.These tests strongly cover success, truncation, failure, and cleanup behavior with clear assertions and isolated mocks.
446-485: Thread-safety/idempotence checks are solid.The focused tests around
_patch_stagehand_once()add meaningful protection against regressions in concurrent contexts.autogpt_platform/backend/backend/copilot/tools/browse_web.py (2)
167-177: Lazy import + graceful Stagehand-absent handling looks good.Scoping optional dependency failure to this tool keeps unrelated tools operational.
201-206: Truncation cap implementation is correct and safe.Nice job ensuring the suffix is included without exceeding
_MAX_CONTENT_CHARS.
| if not url.startswith(("http://", "https://")): | ||
| return ErrorResponse( | ||
| message="Only HTTP/HTTPS URLs are supported.", | ||
| error="invalid_url", | ||
| session_id=session_id, | ||
| ) |
There was a problem hiding this comment.
Strengthen URL validation beyond a raw prefix check.
A startswith check can reject valid mixed-case schemes and accept malformed HTTP URLs without a host. Parse and validate scheme + netloc explicitly.
Suggested fix
import logging
import os
import threading
from typing import Any
+from urllib.parse import urlparse
@@
- if not url.startswith(("http://", "https://")):
+ parsed = urlparse(url)
+ if parsed.scheme.lower() not in {"http", "https"} or not parsed.netloc:
return ErrorResponse(
message="Only HTTP/HTTPS URLs are supported.",
error="invalid_url",
session_id=session_id,
)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@autogpt_platform/backend/backend/copilot/tools/browse_web.py` around lines
136 - 141, The prefix-only URL check is too weak; update the validation in the
browse_web handler (the code that checks the variable `url` and currently
returns `ErrorResponse`) to parse the URL with urllib.parse.urlparse, verify
that parsed.scheme.lower() is either "http" or "https", and ensure parsed.netloc
is non-empty (reject if empty or missing); replace the startswith branch with
this parsed-scheme + netloc check and return the same ErrorResponse
(error="invalid_url") when validation fails so malformed or mixed-case schemes
are rejected and true HTTP/HTTPS URLs are accepted.
autogpt-reviewer
left a comment
autogpt-reviewer
left a comment
There was a problem hiding this comment.
📋 Automated Review — PR #12214
PR #12214 — feat(copilot): Add browse_web tool for JS-rendered page browsing
Author: majdyz | Files: 5 (+728/-0) | HEAD: 1c267998
🎯 Verdict: APPROVE WITH CONDITIONS
What This PR Does
Adds a new browse_web copilot tool backed by Stagehand + Browserbase that enables the AI copilot to navigate and extract content from JavaScript-rendered pages (SPAs, dynamic content). This complements the existing web_fetch tool which only handles static HTTP pages. Each call creates an ephemeral cloud browser session, navigates to a URL, runs LLM-powered extraction, and returns up to 50K chars of content.
Specialist Findings
🛡️ Security startswith("http://") which is weaker than the urlparse + SSRF blocklist approach used by web_fetch (backend/util/request.py:validate_url). However, since browsing happens in Browserbase's cloud (not the AutoGPT backend), there is no SSRF risk to AutoGPT infrastructure. Risk is limited to potential abuse of Browserbase's internal network, which is their sandbox responsibility. Auth enforcement (requires_auth=True) correctly implemented. Error messages sanitized — no secret leakage. Signal handler patch thread-safe. Tests explicitly verify no exception text leaks.
🏗️ Architecture ✅ — Perfect adherence to BaseTool pattern. Dependency direction correct (duplicated 5-line signal patch from blocks/stagehand/ rather than creating cross-layer coupling). Clean module structure at 227 lines. Well-differentiated from web_fetch. Overlap with PRs #12213/#12230 is append-only (low conflict risk).
⚡ Performance ✅ — Per-call latency (10-30s for cloud browser + LLM extraction) is inherent and acceptable for a heavyweight fallback tool. Timeouts properly configured (30s navigation, 60s extraction). 50K char truncation correctly implemented. No client.init() timeout is a minor gap. Thread pool starvation is theoretical (LLM naturally rate-limits calls). Silent except: pass in cleanup should log a warning.
🧪 Testing ✅ — Excellent. 486 lines of tests covering all code paths: metadata, input validation (5 URL cases), env var checks (3 combos), ImportError handling, happy path (7 tests), truncation edge cases (6 boundary tests including exact-at-limit), error handling (5 tests), cleanup verification, and thread-safety of signal patch. Secret leakage explicitly tested. All CI green (lint, types, test 3.11/3.12/3.13, e2e, integration, CodeQL, Snyk).
📖 Quality ✅ — Well-structured, readable code. Constants well-named with _MS suffix convention. Logging appropriate (no secrets). Type hints complete. CodeRabbit's 5 actionable items from round 1 all properly fixed in commit 9ae1bda. Docstring coverage at 26% (below 80% threshold) but consistent with all other tools in the codebase — this is a repo-wide pattern, not a PR-specific gap.
📦 Product ✅ — Fills a genuine gap (JS-rendered pages invisible to web_fetch). Clean invocation via copilot function-calling. instruction parameter enables targeted extraction. Graceful degradation when env vars missing. Stateless v1 scope is appropriate. Should add cost hint to tool description to guide LLM toward cheaper web_fetch when possible.
📬 Discussion ✅ — 5/5 critical items from CodeRabbit round 1 verified fixed. One minor unresolved item from round 2 (URL validation with urlparse — addressed in conditions below). Author's design notes are thorough and well-reasoned. Zero human reviewers so far.
🔎 QA ✅ — Live testing confirmed: frontend loads cleanly, browse_web tool registered and callable via copilot, graceful not_configured error when Stagehand credentials absent, copilot auto-falls back to web_fetch, no console errors, OpenAPI schema consistent. Build page renders without issues.
Conditions (should fix before merge)
-
browse_web.py:136— Strengthen URL validation withurlparse: Replace thestartswithcheck withurllib.parse.urlparse()to verifyscheme in ("http", "https")andnetlocis non-empty. This prevents malformed URLs likehttp://(no host) and handles mixed-case schemes. Consider also blocking obvious private targets (localhost,169.254.x.x,10.x.x.x) for defense-in-depth, even though the browser runs in Browserbase's cloud. This was flagged by both Security and CodeRabbit round 2. -
browse_web.py:91-95— Add cost/fallback guidance to tool description: Include a note like "Each call launches a cloud browser session. Prefer web_fetch for static pages." This helps the LLM make cost-aware tool selection decisions.
Should Fix (Follow-up OK)
browse_web.py:200— Log cleanup failures instead of silentpass: Changeexcept Exception: passtologger.warning("[browse_web] Failed to close client", exc_info=True)for observability.browse_web.py:176— Add timeout onclient.init(): Currently onlygotoandextracthave timeouts. If Browserbase is slow,init()can block indefinitely. Wrap inasyncio.wait_for()or add an overall timeout budget.- Shared utility for Stagehand signal patch: The 5-line monkey-patch is duplicated between
browse_web.pyandblocks/stagehand/blocks.py. Consider extracting tobackend/util/stagehand.py. - Add comment above
_STAGEHAND_MODEL: Document why this specific model version is pinned and when it should be updated.
Risk Assessment
Merge risk: LOW | Rollback: EASY (pure addition, no existing code modified)
All CI green. Pure additive change — 5 new/modified files, zero lines deleted from existing code. Feature is gated behind env vars (STAGEHAND_API_KEY, STAGEHAND_PROJECT_ID) so it's effectively dormant until configured. requires_auth=True prevents anonymous abuse. Rollback is trivial (revert adds).
QA Screenshots
@ntindle Clean, well-tested copilot tool addition. Two small conditions (URL validation + tool description hint), otherwise ready to merge.
|
Superseded by #12230 which includes this |
github-project-automation
Bot
moved this from 🆕 Needs initial review
to ✅ Done
in AutoGPT development kanban
2 participants
Summary
browse_webcopilot tool backed by Stagehand + Browserbase for real-browser page extractionweb_fetchcannot reachSTAGEHAND_API_KEY/STAGEHAND_PROJECT_IDare not configuredWhat changed
browse_web.py— newBrowseWebTool: navigates to a URL, runs Stagehandextract()with a natural-language instruction, returns up to 50K chars of contentmodels.py— addsBROWSE_WEBresponse type andBrowseWebResponsemodeltools/__init__.py— registersbrowse_webinTOOL_REGISTRYDesign decisions
blocks/stagehand/blocks.pyrequires_auth = True— anonymous users cannot consume Browserbase creditsRequired env vars (new)
Test plan
web_fetchstill works independently for static pages