mirror of
https://github.com/actionbook/actionbook.git
synced 2026-09-19 03:28:06 +08:00
93caf967e0
* [packages/dify-plugin]feat: add Dify plugin for browser automation - Implement search_actions and get_action_by_area_id tools - Support API key authentication via Dify plugin SDK - Add comprehensive tests (34 tests, 99% coverage) - Add documentation (README, PRIVACY) - Phase 1: Query action manuals via API Tools: - search_actions: Find website elements by keyword/domain filter - get_action_by_area_id: Get complete selector details by area ID Testing: - Unit tests: provider, search_actions, get_action_by_area_id - Coverage: 99% (412 statements, 3 missed) - Linting: ruff with zero errors Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * [packages/dify-plugin]fix: align SDK interface, fix API issues and improve reliability - Fix API endpoint and parameter alignment with Actionbook API - Align with Dify SDK interface (provider structure, tool signatures) - Restructure provider: replace provider.yaml with provider/ package - Add test infrastructure: unit tests, integration tests, E2E checklist - Improve error handling and remove legacy provider.yaml - Resolve empty response issue in Dify Cloud - Pin dify-plugin>=0.5.1,<0.6.0 to prevent breaking changes - Add *.difypkg, .coverage, .alma-snapshots to root .gitignore * [packages/dify-plugin]fix: address code review issues in source code - C4: Add insecure CDP URL warning for non-localhost ws:// connections - C5: Replace print(stderr) with proper logger in search/get tools - C6: Re-raise exception after logging in HyperbrowserSession.stop() - W8: Make API_BASE_URL configurable via ACTIONBOOK_API_URL env var - W-limit: Fix float limit validation with int() cast - W-except: Narrow bare Exception to PlaywrightTimeout in operator - W-use_proxy: Fix bool comparison for string/bool/whitespace inputs - W7: Document _get_active_page tab selection behavior - Fix type annotation in providers/__init__.py - Update misleading comment in provider/actionbook.py - Add cleanup warning in browser_create_session.yaml * [packages/dify-plugin]fix: correct integration tests to match tool behavior Tools yield error messages instead of raising exceptions. Fix tests that incorrectly used pytest.raises(ValueError) to instead assert on the returned message content. * [packages/dify-plugin]test: add missing unit tests for improved coverage - New test_browser_stop_session.py (6 tests) - Add HTTP 403, SSL/connection error branch tests for search/get tools - Add use_proxy edge cases and default provider tests - Add malformed cdp_url test for browser operator - Add insecure CDP URL warning tests - Update operator tests for PlaywrightTimeout narrowing - Add provider re-raise test 184 tests pass at 86% coverage. * [packages/dify-plugin]test: add E2E browser tests with Hyperbrowser - Add verify_hyperbrowser.py manual verification script - Add 8 E2E tests: tool wrappers (create/stop/navigate/screenshot) and Playwright direct (multi-step, form fill, nav history, HTML) - Add retry logic for intermittent gevent SSL errors - Add tenacity to dev dependencies Run: HYPERBROWSER_API_KEY=hb-xxx uv run pytest -m e2e -v --timeout=120 --no-cov * [packages/dify-plugin]feat: add connection pool, playwright worker and API key resolver - api_key.py: resolve provider key from params or env vars with placeholder filtering - connection_pool.py: in-process pool of browser workers keyed by session_id, using subprocess-based Playwright workers to avoid asyncio conflicts - playwright_worker.py: standalone Playwright worker process supporting CSS, XPath, and snapshot ref selectors (@eN / [ref=eN]) * [packages/dify-plugin]refactor: simplify cdp_client to URL validation only Remove CDP connection logic (cdp_page, _get_active_page, CdpConnectionError) since connections are now managed by the connection pool and playwright worker. Retain only validate_cdp_url for input validation. * [packages/dify-plugin]feat: add snapshot action, connection pool integration and improved error handling browser_operator: - Add snapshot action with accessibility tree (ported from actionbook-rs) - Replace cdp_page with connection pool for persistent sessions - Add SSRF protection for CDP URLs - Improve click/type/fill error messages with recovery hints - Remove screenshot action in favor of snapshot browser_create_session: - Integrate connection pool (pre-connect on session create) - Add profile_id fallback retry logic - Support API key from env vars via resolve_provider_api_key - Validate provider name against SUPPORTED_PROVIDERS browser_stop_session: - Disconnect pooled connection before stopping provider session - Improved error messages with troubleshooting hints hyperbrowser provider: - Add profile_id UUID normalization (uuid5 for non-UUID inputs) actionbook provider: - Add API health check in _validate_credentials * [packages/dify-plugin]fix: improve search/get_action tools with validation and fallback hints - Add area_id character validation regex to prevent injection - Add snapshot fallback guidance in tool descriptions - Reduce debug log verbosity (avoid logging raw parameters) * [packages/dify-plugin]docs: update README workflow guide, manifest version and difyignore - README: add snapshot-based recovery workflow and troubleshooting section - manifest: set version to 0.1.0 (initial release) - difyignore: add scripts/, dist/, .docs/, .DS_Store exclusions * [packages/dify-plugin]chore: remove verify_hyperbrowser script No longer needed — verification is covered by E2E tests. * [packages/dify-plugin]test: update tests for connection pool, snapshot and improved tools - Add test_connection_pool.py: pool connect/disconnect/execute lifecycle - Add test_playwright_worker.py: worker process smoke test - Add test_workflow_snapshot_recovery.py: snapshot-based selector recovery - Update browser_operator tests for snapshot action and error handling - Update browser_create/stop session tests for pool integration - Update cdp_client tests (simplified to URL validation only) - Update provider/search/get_action tests for new validation logic - Replace screenshot references with snapshot in E2E tests * [packages/dify-plugin]refactor: cache provider info in connection pool so stop_session auto-resolves credentials browser_stop_session no longer requires provider/api_key params — they are looked up from the pool via get_session_info(). api_key field in browser_create_session changed to secret-input with form-level input. * [packages/dify-plugin]docs: update README with browser tools and remove unused import - Add documentation for browser_create_session, browser_stop_session, browser_operator tools - Update features list and use cases to reflect browser automation - Remove outdated Phase 2 roadmap (already implemented) - Fix provider names (Hyperbrowser/Steel.dev, not Browserbase) - Note that browser_stop_session auto-resolves provider/api_key - Remove unused SUPPORTED_PROVIDERS import from browser_stop_session.py * [packages/dify-plugin]fix: correct YAML descriptions and add zh_Hans translations - Fix browser_create_session llm_description referencing non-existent tool names (browser_navigate → browser_operator) - Remove Steel.dev mention from descriptions (not yet available) - Update manifest description to reflect browser automation capabilities - Add proper zh_Hans translations for search_actions, get_action_by_area_id, manifest and provider YAML files * [packages/dify-plugin]fix: address CodeQL URL sanitization and logging findings * [packages/dify-plugin]fix: remove dynamic data from error logs * [packages/dify-plugin]feat: add tag-based release workflow and version sync Add automated dify-plugin release pipeline: - release.yml: tag detection (actionbook-dify-plugin-v{VERSION}) and release job that packages .difypkg via uv + creates GitHub Release - sync-versions.js: sync package.json version to manifest.yaml (root + meta.version) and pyproject.toml with exact equality checks and fail-loud on missing fields * [packages/dify-plugin]fix: handle stop/create session failure recovery * [packages/dify-plugin]fix: resolve critical review findings — deadlock, exception safety, timeout handling - Fix connection pool deadlock: move worker.stop() outside pool lock to prevent deadlock when _io_lock is held by another thread in call()/readline(). All call sites (disconnect, disconnect_all, cleanup_stale, connect eviction, get_page health check) now pop under lock and stop outside lock. - Fix BaseException swallowing: re-raise KeyboardInterrupt, SystemExit, and GeneratorExit in search_actions and get_action_by_area_id generators. Only non-Exception BaseException subclasses (e.g. gevent.Timeout) are caught. - Fix timeout exception misclassification: add WorkerTimeoutError exception class. SubprocessWorker.call() now detects timeout errors in worker responses and raises WorkerTimeoutError instead of RuntimeError, so browser_operator handlers can show proper "element not found" hints instead of generic errors. - Document SSRF DNS rebinding TOCTOU limitation in _is_ssrf_target with explanation of cloud browser architecture mitigation. --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
257 lines
8.4 KiB
Python
257 lines
8.4 KiB
Python
"""Tests for the in-process subprocess-worker connection pool."""
|
|
|
|
import sys
|
|
import time
|
|
from pathlib import Path
|
|
from unittest.mock import MagicMock, patch
|
|
|
|
import pytest
|
|
|
|
sys.path.insert(0, str(Path(__file__).parent.parent))
|
|
|
|
from utils.connection_pool import (
|
|
ConnectionNotFound,
|
|
ConnectionPool,
|
|
ConnectionUnhealthy,
|
|
)
|
|
|
|
|
|
def _make_mock_worker():
|
|
"""Create a mock _SubprocessWorker that passes health checks."""
|
|
worker = MagicMock()
|
|
worker._proc = MagicMock()
|
|
worker._proc.poll.return_value = None # process is alive
|
|
worker.call.return_value = 1 # health check evaluate("1") returns 1
|
|
return worker
|
|
|
|
|
|
@pytest.fixture
|
|
def fresh_pool():
|
|
"""Create a fresh ConnectionPool for each test."""
|
|
return ConnectionPool(max_idle_seconds=60)
|
|
|
|
|
|
class TestConnect:
|
|
@patch("utils.connection_pool._SubprocessWorker")
|
|
def test_connect_returns_page(self, MockWorker, fresh_pool):
|
|
mock_worker = _make_mock_worker()
|
|
MockWorker.return_value = mock_worker
|
|
|
|
result = fresh_pool.connect("sess-1", "wss://example.com/ws")
|
|
|
|
assert fresh_pool.size == 1
|
|
assert fresh_pool.has("sess-1")
|
|
MockWorker.assert_called_once_with(
|
|
session_id="sess-1",
|
|
ws_endpoint="wss://example.com/ws",
|
|
timeout_ms=30000,
|
|
)
|
|
|
|
@patch("utils.connection_pool._SubprocessWorker")
|
|
def test_connect_replaces_existing(self, MockWorker, fresh_pool):
|
|
worker1 = _make_mock_worker()
|
|
worker2 = _make_mock_worker()
|
|
MockWorker.side_effect = [worker1, worker2]
|
|
|
|
fresh_pool.connect("sess-1", "wss://old.com/ws")
|
|
fresh_pool.connect("sess-1", "wss://new.com/ws")
|
|
|
|
assert fresh_pool.size == 1
|
|
# Old worker should have been stopped
|
|
worker1.stop.assert_called_once()
|
|
|
|
@patch("utils.connection_pool._SubprocessWorker")
|
|
def test_connect_failure_cleans_up(self, MockWorker, fresh_pool):
|
|
MockWorker.side_effect = ConnectionUnhealthy("startup failed")
|
|
|
|
with pytest.raises(ConnectionUnhealthy, match="startup failed"):
|
|
fresh_pool.connect("sess-1", "wss://bad.com/ws")
|
|
|
|
assert fresh_pool.size == 0
|
|
|
|
@patch("utils.connection_pool._SubprocessWorker")
|
|
def test_connect_custom_timeout(self, MockWorker, fresh_pool):
|
|
mock_worker = _make_mock_worker()
|
|
MockWorker.return_value = mock_worker
|
|
|
|
fresh_pool.connect("sess-1", "wss://example.com/ws", timeout_ms=5000)
|
|
|
|
MockWorker.assert_called_once_with(
|
|
session_id="sess-1",
|
|
ws_endpoint="wss://example.com/ws",
|
|
timeout_ms=5000,
|
|
)
|
|
|
|
@patch("utils.connection_pool._SubprocessWorker")
|
|
def test_connect_rejects_when_pool_full(self, MockWorker):
|
|
pool = ConnectionPool(max_idle_seconds=60, max_size=2)
|
|
MockWorker.return_value = _make_mock_worker()
|
|
|
|
pool.connect("sess-1", "wss://a.com/ws")
|
|
pool.connect("sess-2", "wss://b.com/ws")
|
|
|
|
with pytest.raises(RuntimeError, match="full"):
|
|
pool.connect("sess-3", "wss://c.com/ws")
|
|
|
|
assert pool.size == 2
|
|
|
|
|
|
class TestGetPage:
|
|
@patch("utils.connection_pool._SubprocessWorker")
|
|
def test_get_page_returns_page_proxy(self, MockWorker, fresh_pool):
|
|
mock_worker = _make_mock_worker()
|
|
MockWorker.return_value = mock_worker
|
|
|
|
fresh_pool.connect("sess-1", "wss://example.com/ws")
|
|
result = fresh_pool.get_page("sess-1")
|
|
|
|
# Should return a _PooledPageProxy
|
|
assert result is not None
|
|
|
|
def test_get_page_not_found(self, fresh_pool):
|
|
with pytest.raises(ConnectionNotFound, match="sess-unknown"):
|
|
fresh_pool.get_page("sess-unknown")
|
|
|
|
@patch("utils.connection_pool._SubprocessWorker")
|
|
def test_get_page_unhealthy_removes_connection(self, MockWorker, fresh_pool):
|
|
mock_worker = _make_mock_worker()
|
|
MockWorker.return_value = mock_worker
|
|
|
|
fresh_pool.connect("sess-1", "wss://example.com/ws")
|
|
|
|
# Simulate process exit so health check triggers
|
|
mock_worker._proc.poll.return_value = 1 # exited
|
|
mock_worker.call.side_effect = ConnectionUnhealthy("pipe closed")
|
|
|
|
with pytest.raises(ConnectionUnhealthy, match="dead"):
|
|
fresh_pool.get_page("sess-1")
|
|
|
|
assert fresh_pool.size == 0
|
|
|
|
@patch("utils.connection_pool._SubprocessWorker")
|
|
def test_get_page_updates_last_used(self, MockWorker, fresh_pool):
|
|
mock_worker = _make_mock_worker()
|
|
MockWorker.return_value = mock_worker
|
|
|
|
fresh_pool.connect("sess-1", "wss://example.com/ws")
|
|
|
|
with fresh_pool._lock:
|
|
conn = fresh_pool._connections["sess-1"]
|
|
old_time = conn.last_used_at
|
|
|
|
time.sleep(0.01)
|
|
fresh_pool.get_page("sess-1")
|
|
|
|
with fresh_pool._lock:
|
|
conn = fresh_pool._connections["sess-1"]
|
|
assert conn.last_used_at > old_time
|
|
|
|
|
|
class TestDisconnect:
|
|
@patch("utils.connection_pool._SubprocessWorker")
|
|
def test_disconnect_removes_connection(self, MockWorker, fresh_pool):
|
|
mock_worker = _make_mock_worker()
|
|
MockWorker.return_value = mock_worker
|
|
|
|
fresh_pool.connect("sess-1", "wss://example.com/ws")
|
|
fresh_pool.disconnect("sess-1")
|
|
|
|
assert fresh_pool.size == 0
|
|
assert not fresh_pool.has("sess-1")
|
|
mock_worker.stop.assert_called_once()
|
|
|
|
def test_disconnect_unknown_session_is_noop(self, fresh_pool):
|
|
# Should not raise
|
|
fresh_pool.disconnect("nonexistent")
|
|
|
|
@patch("utils.connection_pool._SubprocessWorker")
|
|
def test_disconnect_all(self, MockWorker, fresh_pool):
|
|
worker1 = _make_mock_worker()
|
|
worker2 = _make_mock_worker()
|
|
MockWorker.side_effect = [worker1, worker2]
|
|
|
|
fresh_pool.connect("sess-1", "wss://a.com/ws")
|
|
fresh_pool.connect("sess-2", "wss://b.com/ws")
|
|
assert fresh_pool.size == 2
|
|
|
|
fresh_pool.disconnect_all()
|
|
|
|
assert fresh_pool.size == 0
|
|
worker1.stop.assert_called_once()
|
|
worker2.stop.assert_called_once()
|
|
|
|
|
|
class TestCleanupStale:
|
|
@patch("utils.connection_pool._SubprocessWorker")
|
|
def test_cleanup_removes_stale_connections(self, MockWorker):
|
|
pool = ConnectionPool(max_idle_seconds=0.01) # 10ms TTL
|
|
MockWorker.return_value = _make_mock_worker()
|
|
|
|
pool.connect("sess-1", "wss://example.com/ws")
|
|
time.sleep(0.02) # Wait for it to become stale
|
|
|
|
pool.cleanup_stale()
|
|
|
|
assert pool.size == 0
|
|
|
|
@patch("utils.connection_pool._SubprocessWorker")
|
|
def test_cleanup_keeps_fresh_connections(self, MockWorker):
|
|
pool = ConnectionPool(max_idle_seconds=60)
|
|
MockWorker.return_value = _make_mock_worker()
|
|
|
|
pool.connect("sess-1", "wss://example.com/ws")
|
|
pool.cleanup_stale()
|
|
|
|
assert pool.size == 1
|
|
|
|
|
|
class TestThreadSafety:
|
|
@patch("utils.connection_pool._SubprocessWorker")
|
|
def test_concurrent_connect_disconnect(self, MockWorker, fresh_pool):
|
|
"""Basic concurrency smoke test with multiple threads."""
|
|
import threading
|
|
|
|
MockWorker.return_value = _make_mock_worker()
|
|
results = []
|
|
|
|
def _worker(sid):
|
|
try:
|
|
fresh_pool.connect(sid, f"wss://example.com/{sid}")
|
|
fresh_pool.get_page(sid)
|
|
fresh_pool.disconnect(sid)
|
|
results.append("ok")
|
|
except Exception as e:
|
|
results.append(str(e))
|
|
|
|
threads = [threading.Thread(target=_worker, args=(f"s-{i}",)) for i in range(5)]
|
|
for t in threads:
|
|
t.start()
|
|
for t in threads:
|
|
t.join()
|
|
|
|
# All threads should succeed (no deadlocks or crashes)
|
|
assert len(results) == 5
|
|
|
|
|
|
class TestEdgeCases:
|
|
@patch("utils.connection_pool._SubprocessWorker")
|
|
def test_disconnect_handles_worker_stop_error(self, MockWorker, fresh_pool):
|
|
mock_worker = _make_mock_worker()
|
|
mock_worker.stop.side_effect = RuntimeError("already stopped")
|
|
MockWorker.return_value = mock_worker
|
|
|
|
fresh_pool.connect("sess-1", "wss://example.com/ws")
|
|
# Should not raise
|
|
fresh_pool.disconnect("sess-1")
|
|
assert fresh_pool.size == 0
|
|
|
|
@patch("utils.connection_pool._SubprocessWorker")
|
|
def test_has_method(self, MockWorker, fresh_pool):
|
|
MockWorker.return_value = _make_mock_worker()
|
|
|
|
assert not fresh_pool.has("sess-1")
|
|
fresh_pool.connect("sess-1", "wss://example.com/ws")
|
|
assert fresh_pool.has("sess-1")
|
|
fresh_pool.disconnect("sess-1")
|
|
assert not fresh_pool.has("sess-1")
|