mirror of
https://github.com/Skyvern-AI/skyvern.git
synced 2026-07-09 16:09:13 +00:00
fix(SKY-11251): repair main-red code-repair-progress seam test after import-strip merge skew (#6749)
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
parent
211de42ea3
commit
778f3828c4
4 changed files with 846 additions and 84 deletions
|
|
@ -441,6 +441,7 @@ _CODE_V2_DEFAULTS: dict[str, Any] = {
|
|||
"run_with": "agent",
|
||||
}
|
||||
_DEFAULT_MCP_PROXY_LOCATION = ProxyLocation.RESIDENTIAL
|
||||
_WORKFLOW_UPDATE_PRESERVED_TOP_LEVEL_FIELDS = ("run_sequentially", "sequential_key")
|
||||
|
||||
|
||||
def _deep_merge(base: Any, override: Any) -> Any:
|
||||
|
|
@ -609,6 +610,28 @@ async def _inject_workflow_update_proxy_default(definition: str, fmt: str, workf
|
|||
return _dump_definition_dict(raw, parsed_format)
|
||||
|
||||
|
||||
async def _inject_workflow_update_top_level_settings(definition: str, fmt: str, workflow_id: str) -> str:
|
||||
"""Preserve workflow-level settings that schema defaults would otherwise clobber."""
|
||||
|
||||
raw, parsed_format = _load_definition_dict(definition, fmt)
|
||||
if raw is None or parsed_format is None:
|
||||
return definition
|
||||
|
||||
missing_fields = [field for field in _WORKFLOW_UPDATE_PRESERVED_TOP_LEVEL_FIELDS if field not in raw]
|
||||
if not missing_fields:
|
||||
return definition
|
||||
|
||||
existing_workflow = await get_workflow_by_id(workflow_id)
|
||||
changed = False
|
||||
for field in missing_fields:
|
||||
existing_value = existing_workflow.get(field)
|
||||
if existing_value is not None:
|
||||
raw[field] = existing_value
|
||||
changed = True
|
||||
|
||||
return _dump_definition_dict(raw, parsed_format) if changed else definition
|
||||
|
||||
|
||||
# Parameter types that are auto-managed (credentials and secrets set via the UI) and should
|
||||
# always be preserved from the existing workflow during MCP updates, regardless of what the
|
||||
# caller sends. These should NEVER be modifiable via MCP — only via the UI credential picker.
|
||||
|
|
@ -1082,6 +1105,7 @@ async def skyvern_workflow_update(
|
|||
|
||||
try:
|
||||
definition = await _inject_workflow_update_proxy_default(definition, format, workflow_id)
|
||||
definition = await _inject_workflow_update_top_level_settings(definition, format, workflow_id)
|
||||
definition = await _inject_workflow_update_parameters(definition, format, workflow_id)
|
||||
except NotFoundError:
|
||||
return make_result(
|
||||
|
|
@ -1094,7 +1118,7 @@ async def skyvern_workflow_update(
|
|||
),
|
||||
)
|
||||
except Exception as e:
|
||||
LOG.warning("workflow_update_proxy_default_injection_failed", workflow_id=workflow_id, error=str(e))
|
||||
LOG.warning("workflow_update_preprocessing_failed", workflow_id=workflow_id, error=str(e))
|
||||
return make_result(
|
||||
"skyvern_workflow_update",
|
||||
ok=False,
|
||||
|
|
|
|||
|
|
@ -2,6 +2,9 @@ import asyncio
|
|||
import copy
|
||||
import hashlib
|
||||
import os
|
||||
import time
|
||||
from collections.abc import AsyncIterator
|
||||
from contextlib import asynccontextmanager
|
||||
from dataclasses import dataclass, field
|
||||
from datetime import timedelta
|
||||
from typing import TYPE_CHECKING, Any, Dict, List
|
||||
|
|
@ -9,6 +12,7 @@ from typing import TYPE_CHECKING, Any, Dict, List
|
|||
import aiohttp
|
||||
import httpx
|
||||
import structlog
|
||||
from cachetools import TTLCache
|
||||
from playwright.async_api import Frame, Page
|
||||
|
||||
from skyvern.config import settings
|
||||
|
|
@ -26,6 +30,7 @@ from skyvern.forge.prompts import prompt_engine
|
|||
from skyvern.forge.sdk.api.aws import AsyncAWSClient
|
||||
from skyvern.forge.sdk.api.azure import AzureClientFactory
|
||||
from skyvern.forge.sdk.api.llm.exceptions import LLMProviderError
|
||||
from skyvern.forge.sdk.cache.base import CACHE_EXPIRE_TIME
|
||||
from skyvern.forge.sdk.copilot.config import CopilotConfig, block_authoring_policy_from_code_only_mode
|
||||
from skyvern.forge.sdk.core import skyvern_context
|
||||
from skyvern.forge.sdk.db.agent_db import AgentDB
|
||||
|
|
@ -53,6 +58,23 @@ USELESS_SHAPE_ATTRIBUTE = [SKYVERN_ID_ATTR, "id", "aria-describedby"]
|
|||
SVG_SHAPE_CONVERTION_ATTEMPTS = 3
|
||||
CSS_SHAPE_CONVERTION_ATTEMPTS = 1
|
||||
INVALID_SHAPE = "N/A"
|
||||
DISABLE_SVG_CONVERT_CACHE_RESILIENCE_FLAG = "DISABLE_SVG_CONVERT_CACHE_RESILIENCE"
|
||||
SVG_LOCAL_CACHE_MAX_ITEMS = 4096
|
||||
SVG_LOCAL_NEGATIVE_CACHE_EXPIRE_TIME = timedelta(hours=1)
|
||||
SVGLocalCacheValue = tuple[str, float | None]
|
||||
|
||||
# TTLCache has one global TTL, so each value also carries an optional shorter
|
||||
# expiry timestamp for negative cache entries.
|
||||
_SVG_LOCAL_SHAPE_CACHE: TTLCache[str, SVGLocalCacheValue] = TTLCache(
|
||||
maxsize=SVG_LOCAL_CACHE_MAX_ITEMS,
|
||||
ttl=CACHE_EXPIRE_TIME.total_seconds(),
|
||||
)
|
||||
# Best-effort single-flight cache: eviction under extreme distinct-key pressure can
|
||||
# allow duplicate conversions, but does not affect conversion correctness.
|
||||
_SVG_CONVERSION_LOCKS: TTLCache[str, asyncio.Lock] = TTLCache(
|
||||
maxsize=SVG_LOCAL_CACHE_MAX_ITEMS,
|
||||
ttl=CACHE_EXPIRE_TIME.total_seconds(),
|
||||
)
|
||||
|
||||
|
||||
@dataclass
|
||||
|
|
@ -115,6 +137,158 @@ def _get_shape_cache_key(hash: str) -> str:
|
|||
return f"skyvern:shape:{hash}"
|
||||
|
||||
|
||||
def _get_svg_conversion_lock(svg_key: str) -> asyncio.Lock:
|
||||
lock = _SVG_CONVERSION_LOCKS.get(svg_key)
|
||||
if lock is None:
|
||||
lock = asyncio.Lock()
|
||||
_SVG_CONVERSION_LOCKS[svg_key] = lock
|
||||
return lock
|
||||
|
||||
|
||||
@asynccontextmanager
|
||||
async def _svg_conversion_lock_scope(svg_key: str, *, use_lock: bool) -> AsyncIterator[None]:
|
||||
if not use_lock:
|
||||
yield
|
||||
return
|
||||
|
||||
async with _get_svg_conversion_lock(svg_key):
|
||||
yield
|
||||
|
||||
|
||||
async def _is_svg_convert_cache_resilience_disabled() -> bool:
|
||||
context = skyvern_context.current()
|
||||
if context is None:
|
||||
return False
|
||||
|
||||
distinct_id = context.run_id or context.workflow_run_id or context.task_id
|
||||
organization_id = context.organization_id
|
||||
if not distinct_id or not organization_id:
|
||||
return False
|
||||
|
||||
experimentation_provider = getattr(app, "EXPERIMENTATION_PROVIDER", None)
|
||||
if not experimentation_provider:
|
||||
return False
|
||||
|
||||
try:
|
||||
flag_enabled = await experimentation_provider.is_feature_enabled_cached(
|
||||
DISABLE_SVG_CONVERT_CACHE_RESILIENCE_FLAG,
|
||||
distinct_id,
|
||||
properties={"organization_id": organization_id},
|
||||
)
|
||||
except Exception:
|
||||
LOG.warning(
|
||||
"Failed to evaluate SVG convert cache resilience flag; defaulting to enabled",
|
||||
exc_info=True,
|
||||
distinct_id=distinct_id,
|
||||
organization_id=organization_id,
|
||||
)
|
||||
return False
|
||||
|
||||
return bool(flag_enabled)
|
||||
|
||||
|
||||
def _get_local_cache_ttl_seconds(svg_shape: str, ex: int | timedelta | None) -> float | None:
|
||||
if ex is None:
|
||||
ttl_seconds = None
|
||||
else:
|
||||
ttl_seconds = ex.total_seconds() if isinstance(ex, timedelta) else float(ex)
|
||||
|
||||
if svg_shape == INVALID_SHAPE:
|
||||
negative_ttl_seconds = SVG_LOCAL_NEGATIVE_CACHE_EXPIRE_TIME.total_seconds()
|
||||
if ttl_seconds is None:
|
||||
return negative_ttl_seconds
|
||||
return min(ttl_seconds, negative_ttl_seconds)
|
||||
return ttl_seconds
|
||||
|
||||
|
||||
def _get_local_cache_expires_at(svg_shape: str, ex: int | timedelta | None) -> float | None:
|
||||
ttl_seconds = _get_local_cache_ttl_seconds(svg_shape, ex)
|
||||
if ttl_seconds is None:
|
||||
return None
|
||||
return time.monotonic() + max(ttl_seconds, 0.0)
|
||||
|
||||
|
||||
def _get_svg_shape_from_local_cache(svg_key: str) -> str | None:
|
||||
cached_value = _SVG_LOCAL_SHAPE_CACHE.get(svg_key)
|
||||
if cached_value is None:
|
||||
return None
|
||||
|
||||
svg_shape, expires_at = cached_value
|
||||
if expires_at is not None and expires_at <= time.monotonic():
|
||||
_SVG_LOCAL_SHAPE_CACHE.pop(svg_key, None)
|
||||
return None
|
||||
return svg_shape
|
||||
|
||||
|
||||
def _cache_svg_shape_locally(
|
||||
svg_key: str,
|
||||
svg_shape: str | None,
|
||||
*,
|
||||
ex: int | timedelta | None = CACHE_EXPIRE_TIME,
|
||||
) -> None:
|
||||
if svg_shape:
|
||||
_SVG_LOCAL_SHAPE_CACHE[svg_key] = (svg_shape, _get_local_cache_expires_at(svg_shape, ex))
|
||||
|
||||
|
||||
async def _get_cached_svg_shape(svg_key: str, *, use_local_cache: bool = True) -> str | None:
|
||||
if use_local_cache:
|
||||
local_shape = _get_svg_shape_from_local_cache(svg_key)
|
||||
if local_shape:
|
||||
return local_shape
|
||||
|
||||
try:
|
||||
cached_svg_shape = await app.CACHE.get(svg_key)
|
||||
except Exception:
|
||||
LOG.warning(
|
||||
"Failed to loaded SVG cache",
|
||||
exc_info=True,
|
||||
key=svg_key,
|
||||
)
|
||||
if use_local_cache:
|
||||
return _get_svg_shape_from_local_cache(svg_key)
|
||||
return None
|
||||
|
||||
svg_shape = cached_svg_shape if isinstance(cached_svg_shape, str) else None
|
||||
if use_local_cache:
|
||||
_cache_svg_shape_locally(svg_key, svg_shape)
|
||||
return svg_shape
|
||||
|
||||
|
||||
async def _set_svg_cache(
|
||||
svg_key: str,
|
||||
svg_shape: str,
|
||||
*,
|
||||
ex: int | timedelta | None = CACHE_EXPIRE_TIME,
|
||||
cache_locally: bool = True,
|
||||
) -> None:
|
||||
if cache_locally:
|
||||
_cache_svg_shape_locally(svg_key, svg_shape, ex=ex)
|
||||
try:
|
||||
await app.CACHE.set(svg_key, svg_shape, ex=ex)
|
||||
except Exception:
|
||||
LOG.warning(
|
||||
"Failed to store SVG cache",
|
||||
exc_info=True,
|
||||
key=svg_key,
|
||||
)
|
||||
|
||||
|
||||
async def _set_css_shape_cache(
|
||||
shape_key: str,
|
||||
css_shape: str,
|
||||
*,
|
||||
ex: int | timedelta | None = CACHE_EXPIRE_TIME,
|
||||
) -> None:
|
||||
try:
|
||||
await app.CACHE.set(shape_key, css_shape, ex=ex)
|
||||
except Exception:
|
||||
LOG.warning(
|
||||
"Failed to store CSS shape cache",
|
||||
exc_info=True,
|
||||
key=shape_key,
|
||||
)
|
||||
|
||||
|
||||
def _remove_skyvern_attributes(element: Dict) -> Dict:
|
||||
"""
|
||||
To get the original HTML element without skyvern attributes
|
||||
|
|
@ -223,99 +397,107 @@ async def _convert_svg_to_string(
|
|||
svg_key = _get_svg_cache_key(svg_hash)
|
||||
|
||||
svg_shape: str | None = None
|
||||
try:
|
||||
svg_shape = await app.CACHE.get(svg_key)
|
||||
except Exception:
|
||||
LOG.warning(
|
||||
"Failed to loaded SVG cache",
|
||||
exc_info=True,
|
||||
key=svg_key,
|
||||
)
|
||||
refresh_svg_cache = False
|
||||
use_cache_resilience = not await _is_svg_convert_cache_resilience_disabled()
|
||||
async with _svg_conversion_lock_scope(svg_key, use_lock=use_cache_resilience):
|
||||
svg_shape = await _get_cached_svg_shape(svg_key, use_local_cache=use_cache_resilience)
|
||||
|
||||
if svg_shape:
|
||||
LOG.debug("SVG loaded from cache", element_id=element_id, key=svg_key, shape=svg_shape)
|
||||
else:
|
||||
if _is_element_already_dropped(svg_key):
|
||||
LOG.debug("SVG is already dropped, going to abort conversion", element_id=element_id, key=svg_key)
|
||||
_mark_element_as_dropped(element, hashed_key=svg_key)
|
||||
return
|
||||
if svg_shape:
|
||||
LOG.debug("SVG loaded from cache", element_id=element_id, key=svg_key, shape=svg_shape)
|
||||
refresh_svg_cache = True
|
||||
else:
|
||||
if _is_element_already_dropped(svg_key):
|
||||
LOG.debug("SVG is already dropped, going to abort conversion", element_id=element_id, key=svg_key)
|
||||
_mark_element_as_dropped(element, hashed_key=svg_key)
|
||||
return
|
||||
|
||||
if len(svg_html) > settings.SVG_MAX_LENGTH:
|
||||
# TODO: implement a fallback solution for "too large" case, maybe convert by screenshot
|
||||
LOG.warning(
|
||||
"SVG element is too large to convert, going to drop the svg element.",
|
||||
element_id=element_id,
|
||||
length=len(svg_html),
|
||||
key=svg_key,
|
||||
)
|
||||
_mark_element_as_dropped(element, hashed_key=svg_key)
|
||||
return
|
||||
|
||||
LOG.debug("call LLM to convert SVG to string shape", element_id=element_id)
|
||||
svg_convert_prompt = prompt_engine.load_prompt("svg-convert", svg_element=svg_html)
|
||||
|
||||
for retry in range(SVG_SHAPE_CONVERTION_ATTEMPTS):
|
||||
try:
|
||||
async with asyncio.timeout(_LLM_CALL_TIMEOUT_SECONDS):
|
||||
if app.SVG_CSS_CONVERTER_LLM_API_HANDLER is None:
|
||||
raise Exception("To enable svg shape conversion, please set the Secondary LLM key")
|
||||
json_response = await app.SVG_CSS_CONVERTER_LLM_API_HANDLER(
|
||||
prompt=svg_convert_prompt, step=step, prompt_name="svg-convert"
|
||||
)
|
||||
svg_shape = json_response.get("shape", "")
|
||||
recognized = json_response.get("recognized", False)
|
||||
if not svg_shape or not recognized:
|
||||
raise Exception("Empty or unrecognized SVG shape replied by secondary llm")
|
||||
LOG.info("SVG converted by LLM", element_id=element_id, key=svg_key, shape=svg_shape)
|
||||
await app.CACHE.set(svg_key, svg_shape)
|
||||
break
|
||||
except LLMProviderError:
|
||||
LOG.info(
|
||||
"Failed to convert SVG to string due to llm error. Will retry if haven't met the max try attempt.",
|
||||
exc_info=True,
|
||||
element_id=element_id,
|
||||
key=svg_key,
|
||||
retry=retry,
|
||||
)
|
||||
if retry == SVG_SHAPE_CONVERTION_ATTEMPTS - 1:
|
||||
# set the invalid css shape to cache to avoid retry in the near future
|
||||
await app.CACHE.set(svg_key, INVALID_SHAPE, ex=timedelta(hours=1))
|
||||
else:
|
||||
await asyncio.sleep(0.5 * (2**retry))
|
||||
except asyncio.TimeoutError:
|
||||
if len(svg_html) > settings.SVG_MAX_LENGTH:
|
||||
# TODO: implement a fallback solution for "too large" case, maybe convert by screenshot
|
||||
LOG.warning(
|
||||
"Timeout to call LLM to parse SVG. Going to drop the svg element directly.",
|
||||
"SVG element is too large to convert, going to drop the svg element.",
|
||||
element_id=element_id,
|
||||
length=len(svg_html),
|
||||
key=svg_key,
|
||||
)
|
||||
_mark_element_as_dropped(element, hashed_key=svg_key)
|
||||
return
|
||||
except Exception:
|
||||
LOG.info(
|
||||
"Failed to convert SVG to string shape by secondary llm. Will retry if haven't met the max try attempt.",
|
||||
exc_info=True,
|
||||
|
||||
LOG.debug("call LLM to convert SVG to string shape", element_id=element_id)
|
||||
svg_convert_prompt = prompt_engine.load_prompt("svg-convert", svg_element=svg_html)
|
||||
|
||||
for retry in range(SVG_SHAPE_CONVERTION_ATTEMPTS):
|
||||
try:
|
||||
async with asyncio.timeout(_LLM_CALL_TIMEOUT_SECONDS):
|
||||
if app.SVG_CSS_CONVERTER_LLM_API_HANDLER is None:
|
||||
raise Exception("To enable svg shape conversion, please set the Secondary LLM key")
|
||||
json_response = await app.SVG_CSS_CONVERTER_LLM_API_HANDLER(
|
||||
prompt=svg_convert_prompt, step=step, prompt_name="svg-convert"
|
||||
)
|
||||
svg_shape = json_response.get("shape", "")
|
||||
recognized = json_response.get("recognized", False)
|
||||
if not svg_shape or not recognized:
|
||||
raise Exception("Empty or unrecognized SVG shape replied by secondary llm")
|
||||
LOG.info("SVG converted by LLM", element_id=element_id, key=svg_key, shape=svg_shape)
|
||||
await _set_svg_cache(svg_key, svg_shape, cache_locally=use_cache_resilience)
|
||||
break
|
||||
except LLMProviderError:
|
||||
LOG.info(
|
||||
"Failed to convert SVG to string due to llm error. Will retry if haven't met the max try attempt.",
|
||||
exc_info=True,
|
||||
element_id=element_id,
|
||||
key=svg_key,
|
||||
retry=retry,
|
||||
)
|
||||
if retry == SVG_SHAPE_CONVERTION_ATTEMPTS - 1:
|
||||
# set the invalid css shape to cache to avoid retry in the near future
|
||||
await _set_svg_cache(
|
||||
svg_key,
|
||||
INVALID_SHAPE,
|
||||
ex=timedelta(hours=1),
|
||||
cache_locally=use_cache_resilience,
|
||||
)
|
||||
else:
|
||||
await asyncio.sleep(0.5 * (2**retry))
|
||||
except asyncio.TimeoutError:
|
||||
LOG.warning(
|
||||
"Timeout to call LLM to parse SVG. Going to drop the svg element directly.",
|
||||
element_id=element_id,
|
||||
key=svg_key,
|
||||
)
|
||||
_mark_element_as_dropped(element, hashed_key=svg_key)
|
||||
return
|
||||
except Exception:
|
||||
LOG.info(
|
||||
"Failed to convert SVG to string shape by secondary llm. Will retry if haven't met the max try attempt.",
|
||||
exc_info=True,
|
||||
element_id=element_id,
|
||||
retry=retry,
|
||||
)
|
||||
if retry == SVG_SHAPE_CONVERTION_ATTEMPTS - 1:
|
||||
# set the invalid css shape to cache to avoid retry in the near future
|
||||
await _set_svg_cache(
|
||||
svg_key,
|
||||
INVALID_SHAPE,
|
||||
ex=timedelta(weeks=1),
|
||||
cache_locally=use_cache_resilience,
|
||||
)
|
||||
else:
|
||||
await asyncio.sleep(0.5 * (2**retry))
|
||||
else:
|
||||
LOG.warning(
|
||||
"Reaching the max try to convert svg element, going to drop the svg element.",
|
||||
element_id=element_id,
|
||||
retry=retry,
|
||||
key=svg_key,
|
||||
length=len(svg_html),
|
||||
)
|
||||
if retry == SVG_SHAPE_CONVERTION_ATTEMPTS - 1:
|
||||
# set the invalid css shape to cache to avoid retry in the near future
|
||||
await app.CACHE.set(svg_key, INVALID_SHAPE, ex=timedelta(weeks=1))
|
||||
else:
|
||||
await asyncio.sleep(0.5 * (2**retry))
|
||||
else:
|
||||
LOG.warning(
|
||||
"Reaching the max try to convert svg element, going to drop the svg element.",
|
||||
element_id=element_id,
|
||||
key=svg_key,
|
||||
length=len(svg_html),
|
||||
)
|
||||
_mark_element_as_dropped(element, hashed_key=svg_key)
|
||||
return
|
||||
_mark_element_as_dropped(element, hashed_key=svg_key)
|
||||
return
|
||||
|
||||
element["attributes"] = dict()
|
||||
if svg_shape != INVALID_SHAPE:
|
||||
# refresh the cache expiration
|
||||
await app.CACHE.set(svg_key, svg_shape)
|
||||
if refresh_svg_cache:
|
||||
await _set_svg_cache(svg_key, svg_shape, cache_locally=use_cache_resilience)
|
||||
element["attributes"]["alt"] = svg_shape
|
||||
if "children" in element:
|
||||
del element["children"]
|
||||
|
|
@ -404,7 +586,7 @@ async def _convert_css_shape_to_string(
|
|||
if not css_shape or not recognized:
|
||||
raise Exception("Empty or unrecognized css shape replied by secondary llm")
|
||||
LOG.info("CSS Shape converted by LLM", element_id=element_id, key=shape_key, shape=css_shape)
|
||||
await app.CACHE.set(shape_key, css_shape)
|
||||
await _set_css_shape_cache(shape_key, css_shape)
|
||||
break
|
||||
except LLMProviderError:
|
||||
LOG.info(
|
||||
|
|
@ -416,7 +598,7 @@ async def _convert_css_shape_to_string(
|
|||
)
|
||||
if retry == CSS_SHAPE_CONVERTION_ATTEMPTS - 1:
|
||||
# set the invalid css shape to cache to avoid retry in the near future
|
||||
await app.CACHE.set(shape_key, INVALID_SHAPE, ex=timedelta(hours=1))
|
||||
await _set_css_shape_cache(shape_key, INVALID_SHAPE, ex=timedelta(hours=1))
|
||||
except asyncio.TimeoutError:
|
||||
LOG.warning(
|
||||
"Timeout to call LLM to parse css shape. Going to abort the convertion directly.",
|
||||
|
|
@ -435,7 +617,7 @@ async def _convert_css_shape_to_string(
|
|||
)
|
||||
if retry == CSS_SHAPE_CONVERTION_ATTEMPTS - 1:
|
||||
# set the invalid css shape to cache to avoid retry in the near future
|
||||
await app.CACHE.set(shape_key, INVALID_SHAPE, ex=timedelta(weeks=1))
|
||||
await _set_css_shape_cache(shape_key, INVALID_SHAPE, ex=timedelta(weeks=1))
|
||||
else:
|
||||
LOG.info(
|
||||
"Max css shape convertion retry, going to abort the convertion.",
|
||||
|
|
@ -458,7 +640,7 @@ async def _convert_css_shape_to_string(
|
|||
element["attributes"] = dict()
|
||||
if css_shape != INVALID_SHAPE:
|
||||
# refresh the cache expiration
|
||||
await app.CACHE.set(shape_key, css_shape)
|
||||
await _set_css_shape_cache(shape_key, css_shape)
|
||||
element["attributes"]["shape-description"] = css_shape
|
||||
return None
|
||||
|
||||
|
|
|
|||
363
tests/unit/forge/test_svg_convert_cache_resilience.py
Normal file
363
tests/unit/forge/test_svg_convert_cache_resilience.py
Normal file
|
|
@ -0,0 +1,363 @@
|
|||
import asyncio
|
||||
import copy
|
||||
import hashlib
|
||||
from collections.abc import Generator
|
||||
from datetime import timedelta
|
||||
from types import SimpleNamespace
|
||||
from typing import Any
|
||||
from unittest.mock import AsyncMock
|
||||
|
||||
import pytest
|
||||
|
||||
from skyvern.forge import agent_functions
|
||||
from skyvern.forge.sdk.cache.base import CACHE_EXPIRE_TIME
|
||||
from skyvern.forge.sdk.core import skyvern_context
|
||||
|
||||
|
||||
def _svg_element() -> dict[str, Any]:
|
||||
return {
|
||||
"tagName": "svg",
|
||||
"id": "AAAK",
|
||||
"attributes": {"id": "AAAK"},
|
||||
"children": [{"tagName": "path", "attributes": {"d": "M0 0h10v10z"}}],
|
||||
}
|
||||
|
||||
|
||||
def _svg_cache_key(element: dict[str, Any]) -> str:
|
||||
svg_element = agent_functions._remove_skyvern_attributes(element)
|
||||
svg_html = agent_functions.json_to_html(svg_element)
|
||||
return agent_functions._get_svg_cache_key(hashlib.sha256(svg_html.encode("utf-8")).hexdigest())
|
||||
|
||||
|
||||
class _FailingCache:
|
||||
async def get(self, key: str) -> Any:
|
||||
raise ConnectionError("redis unavailable")
|
||||
|
||||
async def set(self, key: str, value: Any, ex: Any = CACHE_EXPIRE_TIME) -> None:
|
||||
raise ConnectionError("redis unavailable")
|
||||
|
||||
|
||||
class _RecordingFailingCache(_FailingCache):
|
||||
def __init__(self) -> None:
|
||||
self.set_calls: list[tuple[str, Any, Any]] = []
|
||||
|
||||
async def set(self, key: str, value: Any, ex: Any = CACHE_EXPIRE_TIME) -> None:
|
||||
self.set_calls.append((key, value, ex))
|
||||
await super().set(key, value, ex=ex)
|
||||
|
||||
|
||||
class _MemoryCache:
|
||||
def __init__(self) -> None:
|
||||
self.values: dict[str, Any] = {}
|
||||
self.fail_get = False
|
||||
|
||||
async def get(self, key: str) -> Any:
|
||||
if self.fail_get:
|
||||
raise ConnectionError("redis get unavailable")
|
||||
return self.values.get(key)
|
||||
|
||||
async def set(self, key: str, value: Any, ex: Any = CACHE_EXPIRE_TIME) -> None:
|
||||
self.values[key] = value
|
||||
|
||||
|
||||
class _SetFailingMemoryCache:
|
||||
async def get(self, key: str) -> Any:
|
||||
return None
|
||||
|
||||
async def set(self, key: str, value: Any, ex: Any = CACHE_EXPIRE_TIME) -> None:
|
||||
raise ConnectionError("redis set unavailable")
|
||||
|
||||
|
||||
class _FakeLocator:
|
||||
async def count(self) -> int:
|
||||
return 1
|
||||
|
||||
async def element_handle(self, timeout: float) -> object:
|
||||
return object()
|
||||
|
||||
async def scroll_into_view_if_needed(self, timeout: float) -> None:
|
||||
return None
|
||||
|
||||
async def wait_for(self, state: str, timeout: float) -> None:
|
||||
return None
|
||||
|
||||
async def screenshot(self, timeout: float, animations: str) -> bytes:
|
||||
return b"fake-png"
|
||||
|
||||
|
||||
class _FakeFrame:
|
||||
def locator(self, selector: str) -> _FakeLocator:
|
||||
return _FakeLocator()
|
||||
|
||||
|
||||
class _FakeSkyvernFrame:
|
||||
def get_frame(self) -> _FakeFrame:
|
||||
return _FakeFrame()
|
||||
|
||||
async def get_blocking_element_id(self, element: object) -> tuple[None, bool]:
|
||||
return None, False
|
||||
|
||||
|
||||
def test_svg_local_invalid_shape_cache_uses_short_ttl(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
monkeypatch.setattr(agent_functions.time, "monotonic", lambda: 100.0)
|
||||
|
||||
agent_functions._cache_svg_shape_locally(
|
||||
"svg-key",
|
||||
agent_functions.INVALID_SHAPE,
|
||||
ex=timedelta(weeks=1),
|
||||
)
|
||||
|
||||
assert agent_functions._SVG_LOCAL_SHAPE_CACHE["svg-key"] == (
|
||||
agent_functions.INVALID_SHAPE,
|
||||
100.0 + agent_functions.SVG_LOCAL_NEGATIVE_CACHE_EXPIRE_TIME.total_seconds(),
|
||||
)
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _reset_svg_convert_state(monkeypatch: pytest.MonkeyPatch) -> Generator[None, None, None]:
|
||||
agent_functions._SVG_LOCAL_SHAPE_CACHE.clear()
|
||||
agent_functions._SVG_CONVERSION_LOCKS.clear()
|
||||
skyvern_context.set(skyvern_context.SkyvernContext())
|
||||
monkeypatch.setattr(agent_functions.prompt_engine, "load_prompt", lambda *args, **kwargs: "svg prompt")
|
||||
yield
|
||||
skyvern_context.reset()
|
||||
agent_functions._SVG_LOCAL_SHAPE_CACHE.clear()
|
||||
agent_functions._SVG_CONVERSION_LOCKS.clear()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_svg_convert_does_not_retry_llm_when_cache_set_fails(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
calls = 0
|
||||
|
||||
async def handler(**kwargs: Any) -> dict[str, Any]:
|
||||
nonlocal calls
|
||||
calls += 1
|
||||
return {"shape": "search icon", "recognized": True}
|
||||
|
||||
monkeypatch.setattr(
|
||||
agent_functions,
|
||||
"app",
|
||||
SimpleNamespace(CACHE=_FailingCache(), SVG_CSS_CONVERTER_LLM_API_HANDLER=handler),
|
||||
)
|
||||
|
||||
element = _svg_element()
|
||||
await agent_functions._convert_svg_to_string(element)
|
||||
|
||||
assert calls == 1
|
||||
assert element["attributes"] == {"alt": "search icon"}
|
||||
assert "children" not in element
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_svg_convert_uses_local_fallback_when_redis_is_unavailable(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
calls = 0
|
||||
|
||||
async def handler(**kwargs: Any) -> dict[str, Any]:
|
||||
nonlocal calls
|
||||
calls += 1
|
||||
return {"shape": "search icon", "recognized": True}
|
||||
|
||||
monkeypatch.setattr(
|
||||
agent_functions,
|
||||
"app",
|
||||
SimpleNamespace(CACHE=_FailingCache(), SVG_CSS_CONVERTER_LLM_API_HANDLER=handler),
|
||||
)
|
||||
|
||||
first = _svg_element()
|
||||
second = _svg_element()
|
||||
await agent_functions._convert_svg_to_string(first)
|
||||
await agent_functions._convert_svg_to_string(second)
|
||||
|
||||
assert calls == 1
|
||||
assert first["attributes"] == {"alt": "search icon"}
|
||||
assert second["attributes"] == {"alt": "search icon"}
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_svg_convert_caches_invalid_shape_locally_when_redis_is_unavailable(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
calls = 0
|
||||
|
||||
async def handler(**kwargs: Any) -> dict[str, Any]:
|
||||
nonlocal calls
|
||||
calls += 1
|
||||
return {"shape": "", "recognized": False}
|
||||
|
||||
monkeypatch.setattr(
|
||||
agent_functions,
|
||||
"app",
|
||||
SimpleNamespace(CACHE=_FailingCache(), SVG_CSS_CONVERTER_LLM_API_HANDLER=handler),
|
||||
)
|
||||
|
||||
first = _svg_element()
|
||||
await agent_functions._convert_svg_to_string(first)
|
||||
|
||||
assert calls == 3
|
||||
assert first["isDropped"] is True
|
||||
assert "children" not in first
|
||||
|
||||
skyvern_context.set(skyvern_context.SkyvernContext())
|
||||
second = _svg_element()
|
||||
await agent_functions._convert_svg_to_string(second)
|
||||
|
||||
assert calls == 3
|
||||
assert second["attributes"] == {}
|
||||
assert "children" not in second
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_svg_convert_caches_invalid_shape_loaded_from_redis_locally(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
calls = 0
|
||||
|
||||
async def handler(**kwargs: Any) -> dict[str, Any]:
|
||||
nonlocal calls
|
||||
calls += 1
|
||||
return {"shape": "search icon", "recognized": True}
|
||||
|
||||
cache = _MemoryCache()
|
||||
cache.values[_svg_cache_key(_svg_element())] = agent_functions.INVALID_SHAPE
|
||||
monkeypatch.setattr(
|
||||
agent_functions,
|
||||
"app",
|
||||
SimpleNamespace(CACHE=cache, SVG_CSS_CONVERTER_LLM_API_HANDLER=handler),
|
||||
)
|
||||
|
||||
first = _svg_element()
|
||||
await agent_functions._convert_svg_to_string(first)
|
||||
|
||||
assert calls == 0
|
||||
assert first["attributes"] == {}
|
||||
assert "children" not in first
|
||||
|
||||
cache.fail_get = True
|
||||
skyvern_context.set(skyvern_context.SkyvernContext())
|
||||
second = _svg_element()
|
||||
await agent_functions._convert_svg_to_string(second)
|
||||
|
||||
assert calls == 0
|
||||
assert second["attributes"] == {}
|
||||
assert "children" not in second
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_svg_convert_single_flights_concurrent_duplicate_misses(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
calls = 0
|
||||
|
||||
async def handler(**kwargs: Any) -> dict[str, Any]:
|
||||
nonlocal calls
|
||||
calls += 1
|
||||
return {"shape": "search icon", "recognized": True}
|
||||
|
||||
monkeypatch.setattr(
|
||||
agent_functions,
|
||||
"app",
|
||||
SimpleNamespace(CACHE=_MemoryCache(), SVG_CSS_CONVERTER_LLM_API_HANDLER=handler),
|
||||
)
|
||||
|
||||
elements = [copy.deepcopy(_svg_element()) for _ in range(5)]
|
||||
await asyncio.gather(*[agent_functions._convert_svg_to_string(element) for element in elements])
|
||||
|
||||
assert calls == 1
|
||||
assert all(element["attributes"] == {"alt": "search icon"} for element in elements)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_svg_convert_disable_flag_bypasses_local_cache_and_single_flight(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
calls = 0
|
||||
|
||||
async def handler(**kwargs: Any) -> dict[str, Any]:
|
||||
nonlocal calls
|
||||
calls += 1
|
||||
await asyncio.sleep(0)
|
||||
return {"shape": "search icon", "recognized": True}
|
||||
|
||||
provider = SimpleNamespace(is_feature_enabled_cached=AsyncMock(return_value=True))
|
||||
cache = _RecordingFailingCache()
|
||||
monkeypatch.setattr(
|
||||
agent_functions,
|
||||
"app",
|
||||
SimpleNamespace(
|
||||
CACHE=cache,
|
||||
SVG_CSS_CONVERTER_LLM_API_HANDLER=handler,
|
||||
EXPERIMENTATION_PROVIDER=provider,
|
||||
),
|
||||
)
|
||||
skyvern_context.set(skyvern_context.SkyvernContext(run_id="wr_1", organization_id="o_1"))
|
||||
|
||||
elements = [copy.deepcopy(_svg_element()) for _ in range(5)]
|
||||
await asyncio.gather(*[agent_functions._convert_svg_to_string(element) for element in elements])
|
||||
|
||||
assert calls == len(elements)
|
||||
assert len(cache.set_calls) == len(elements)
|
||||
assert all(element["attributes"] == {"alt": "search icon"} for element in elements)
|
||||
provider.is_feature_enabled_cached.assert_any_await(
|
||||
agent_functions.DISABLE_SVG_CONVERT_CACHE_RESILIENCE_FLAG,
|
||||
"wr_1",
|
||||
properties={"organization_id": "o_1"},
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_svg_convert_flag_error_defaults_to_cache_resilience(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
calls = 0
|
||||
|
||||
async def handler(**kwargs: Any) -> dict[str, Any]:
|
||||
nonlocal calls
|
||||
calls += 1
|
||||
return {"shape": "search icon", "recognized": True}
|
||||
|
||||
provider = SimpleNamespace(is_feature_enabled_cached=AsyncMock(side_effect=RuntimeError("posthog down")))
|
||||
monkeypatch.setattr(
|
||||
agent_functions,
|
||||
"app",
|
||||
SimpleNamespace(
|
||||
CACHE=_FailingCache(),
|
||||
SVG_CSS_CONVERTER_LLM_API_HANDLER=handler,
|
||||
EXPERIMENTATION_PROVIDER=provider,
|
||||
),
|
||||
)
|
||||
skyvern_context.set(skyvern_context.SkyvernContext(run_id="wr_1", organization_id="o_1"))
|
||||
|
||||
first = _svg_element()
|
||||
second = _svg_element()
|
||||
await agent_functions._convert_svg_to_string(first)
|
||||
await agent_functions._convert_svg_to_string(second)
|
||||
|
||||
assert calls == 1
|
||||
assert first["attributes"] == {"alt": "search icon"}
|
||||
assert second["attributes"] == {"alt": "search icon"}
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_css_shape_convert_keeps_result_when_cache_set_fails(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
calls = 0
|
||||
|
||||
async def handler(**kwargs: Any) -> dict[str, Any]:
|
||||
nonlocal calls
|
||||
calls += 1
|
||||
return {"shape": "calendar icon", "recognized": True}
|
||||
|
||||
monkeypatch.setattr(
|
||||
agent_functions,
|
||||
"app",
|
||||
SimpleNamespace(CACHE=_SetFailingMemoryCache(), SVG_CSS_CONVERTER_LLM_API_HANDLER=handler),
|
||||
)
|
||||
|
||||
element: dict[str, Any] = {
|
||||
"tagName": "span",
|
||||
"id": "AAAK",
|
||||
"attributes": {"id": "AAAK"},
|
||||
}
|
||||
await agent_functions._convert_css_shape_to_string(_FakeSkyvernFrame(), element)
|
||||
|
||||
assert calls == 1
|
||||
assert element["attributes"]["shape-description"] == "calendar icon"
|
||||
|
|
@ -507,6 +507,199 @@ async def test_workflow_update_defaults_proxy_when_existing_is_null(monkeypatch:
|
|||
assert sent_definition.get("proxy_location") == "RESIDENTIAL"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_workflow_update_preserves_sequential_settings_when_omitted(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
request_mock = _patch_skyvern_http(monkeypatch, response_payload=_fake_workflow_dict())
|
||||
|
||||
async def fake_get_workflow_by_id(workflow_id: str, version: int | None = None) -> dict[str, object]:
|
||||
assert workflow_id == "wpid_test"
|
||||
assert version is None
|
||||
return {
|
||||
"proxy_location": "RESIDENTIAL",
|
||||
"run_sequentially": True,
|
||||
"sequential_key": "existing-sequential-key",
|
||||
"workflow_definition": {
|
||||
"parameters": [],
|
||||
"blocks": [],
|
||||
},
|
||||
}
|
||||
|
||||
_patch_get_workflow_by_id(monkeypatch, fake_get_workflow_by_id)
|
||||
|
||||
definition = {
|
||||
"title": "Updated workflow",
|
||||
"workflow_definition": {
|
||||
"parameters": [],
|
||||
"blocks": [
|
||||
{
|
||||
"block_type": "navigation",
|
||||
"label": "visit",
|
||||
"url": "https://example.com",
|
||||
"title": "Visit",
|
||||
"navigation_goal": "Open the page",
|
||||
}
|
||||
],
|
||||
},
|
||||
}
|
||||
|
||||
result = await workflow_tools.skyvern_workflow_update(
|
||||
workflow_id="wpid_test",
|
||||
definition=json.dumps(definition),
|
||||
format="json",
|
||||
)
|
||||
|
||||
sent_definition = request_mock.await_args.kwargs["json"]["json_definition"]
|
||||
assert result["ok"] is True
|
||||
assert sent_definition.get("run_sequentially") is True
|
||||
assert sent_definition.get("sequential_key") == "existing-sequential-key"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_workflow_update_respects_explicit_run_sequentially_false(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
request_mock = _patch_skyvern_http(monkeypatch, response_payload=_fake_workflow_dict())
|
||||
|
||||
async def fake_get_workflow_by_id(workflow_id: str, version: int | None = None) -> dict[str, object]:
|
||||
assert workflow_id == "wpid_test"
|
||||
assert version is None
|
||||
return {
|
||||
"proxy_location": "RESIDENTIAL",
|
||||
"run_sequentially": True,
|
||||
"sequential_key": "existing-sequential-key",
|
||||
"workflow_definition": {
|
||||
"parameters": [],
|
||||
"blocks": [],
|
||||
},
|
||||
}
|
||||
|
||||
_patch_get_workflow_by_id(monkeypatch, fake_get_workflow_by_id)
|
||||
|
||||
definition = {
|
||||
"title": "Updated workflow",
|
||||
"run_sequentially": False,
|
||||
"workflow_definition": {
|
||||
"parameters": [],
|
||||
"blocks": [
|
||||
{
|
||||
"block_type": "navigation",
|
||||
"label": "visit",
|
||||
"url": "https://example.com",
|
||||
"title": "Visit",
|
||||
"navigation_goal": "Open the page",
|
||||
}
|
||||
],
|
||||
},
|
||||
}
|
||||
|
||||
result = await workflow_tools.skyvern_workflow_update(
|
||||
workflow_id="wpid_test",
|
||||
definition=json.dumps(definition),
|
||||
format="json",
|
||||
)
|
||||
|
||||
sent_definition = request_mock.await_args.kwargs["json"]["json_definition"]
|
||||
assert result["ok"] is True
|
||||
assert sent_definition.get("run_sequentially") is False
|
||||
assert sent_definition.get("sequential_key") == "existing-sequential-key"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_workflow_update_preserves_overlap_and_credentials_via_mcp_client(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""Exercise workflow update through the registered FastMCP tool boundary."""
|
||||
|
||||
request_mock = _patch_skyvern_http(monkeypatch, response_payload=_fake_workflow_dict())
|
||||
|
||||
async def fake_get_workflow_by_id(workflow_id: str, version: int | None = None) -> dict[str, object]:
|
||||
assert workflow_id == "wpid_test"
|
||||
assert version is None
|
||||
return {
|
||||
"proxy_location": "RESIDENTIAL",
|
||||
"run_sequentially": True,
|
||||
"sequential_key": "existing-sequential-key",
|
||||
"workflow_definition": {
|
||||
"parameters": [
|
||||
{
|
||||
"parameter_type": "credential",
|
||||
"key": "credentials",
|
||||
"credential_id": "cred_abc123",
|
||||
"credential_parameter_id": "cp_xyz",
|
||||
"workflow_id": "wf_test",
|
||||
},
|
||||
{
|
||||
"parameter_type": "workflow",
|
||||
"key": "url_input",
|
||||
"workflow_parameter_type": "string",
|
||||
"default_value": "https://example.com",
|
||||
},
|
||||
],
|
||||
"blocks": [
|
||||
{
|
||||
"block_type": "login",
|
||||
"label": "login_block",
|
||||
"parameter_keys": ["credentials"],
|
||||
}
|
||||
],
|
||||
},
|
||||
}
|
||||
|
||||
_patch_get_workflow_by_id(monkeypatch, fake_get_workflow_by_id)
|
||||
|
||||
definition = {
|
||||
"title": "Updated workflow",
|
||||
"workflow_definition": {
|
||||
"parameters": [
|
||||
{
|
||||
"parameter_type": "workflow",
|
||||
"key": "url_input",
|
||||
"workflow_parameter_type": "string",
|
||||
"default_value": "https://new-url.com",
|
||||
},
|
||||
],
|
||||
"blocks": [
|
||||
{
|
||||
"block_type": "login",
|
||||
"label": "login_block",
|
||||
"parameter_keys": [],
|
||||
"navigation_goal": "Login to the site",
|
||||
}
|
||||
],
|
||||
},
|
||||
}
|
||||
|
||||
async with Client(mcp) as client:
|
||||
result = await client.call_tool(
|
||||
"skyvern_workflow_update",
|
||||
{
|
||||
"workflow_id": "wpid_test",
|
||||
"definition": json.dumps(definition),
|
||||
"format": "json",
|
||||
},
|
||||
)
|
||||
|
||||
assert result.is_error is False
|
||||
assert isinstance(result.data, dict)
|
||||
assert result.data["ok"] is True
|
||||
|
||||
sent_definition = request_mock.await_args.kwargs["json"]["json_definition"]
|
||||
assert sent_definition.get("run_sequentially") is True
|
||||
assert sent_definition.get("sequential_key") == "existing-sequential-key"
|
||||
|
||||
params = sent_definition["workflow_definition"]["parameters"]
|
||||
cred_params = [p for p in params if p.get("parameter_type") == "credential"]
|
||||
assert len(cred_params) == 1
|
||||
assert cred_params[0]["key"] == "credentials"
|
||||
assert cred_params[0]["credential_id"] == "cred_abc123"
|
||||
|
||||
blocks = sent_definition["workflow_definition"]["blocks"]
|
||||
login_block = next(b for b in blocks if b.get("label") == "login_block")
|
||||
assert "credentials" in login_block.get("parameter_keys", [])
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_workflow_create_falls_back_on_schema_validation_error(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
"""If the internal schema rejects the payload, normalization is skipped and the raw dict is forwarded."""
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue