diff --git a/openhands/automation/capabilities_router.py b/openhands/automation/capabilities_router.py index b48dc646..2062c647 100644 --- a/openhands/automation/capabilities_router.py +++ b/openhands/automation/capabilities_router.py @@ -245,7 +245,12 @@ async def _custom_sources(org_id: uuid.UUID, session: AsyncSession) -> list[str] def _cron_interval_floor() -> int: """Shortest interval between fires the scheduler can actually honour.""" - return max(POLL_INTERVAL_SECONDS, get_config().service.scheduler_interval_seconds) + settings = get_config().service + return max( + POLL_INTERVAL_SECONDS, + settings.scheduler_interval_seconds, + settings.min_cron_interval_seconds, + ) def _cron_errors(trigger: CronTrigger) -> list[DraftValidationError]: diff --git a/openhands/automation/config.py b/openhands/automation/config.py index 6e7acfb6..212eaa18 100644 --- a/openhands/automation/config.py +++ b/openhands/automation/config.py @@ -502,6 +502,8 @@ class ServiceSettings(BaseSettings): AUTOMATION_WORKSPACE_BASE: Base workspace directory (local mode default) # Background workers + AUTOMATION_MIN_CRON_INTERVAL_SECONDS: Minimum allowed gap between cron + fire times (default: 0, disabled) AUTOMATION_SCHEDULER_INTERVAL_SECONDS: Scheduler poll interval (default: 60) AUTOMATION_SCHEDULER_BATCH_SIZE: Scheduler batch size (default: 50) AUTOMATION_DISPATCHER_INTERVAL_SECONDS: Dispatcher poll interval (default: 10) @@ -604,6 +606,7 @@ class ServiceSettings(BaseSettings): # Background workers scheduler_interval_seconds: int = 60 scheduler_batch_size: int = 50 + min_cron_interval_seconds: int = Field(default=0, ge=0) dispatcher_interval_seconds: int = 10 dispatcher_batch_size: int = 10 watchdog_interval_seconds: int = 60 diff --git a/openhands/automation/schemas.py b/openhands/automation/schemas.py index 50db7024..7dc5f5aa 100644 --- a/openhands/automation/schemas.py +++ b/openhands/automation/schemas.py @@ -18,6 +18,7 @@ model_validator, ) from pydantic.alias_generators import to_camel +from pydantic_core import PydanticCustomError from openhands.automation.constants import MODEL_PROFILE_PATTERN from openhands.automation.models import AutomationState @@ -28,6 +29,7 @@ verifier_schemes, ) from openhands.automation.utils.cron import ( + min_interval_seconds, validate_cron_schedule as validate_cron_schedule_value, validate_timezone_name, ) @@ -68,7 +70,19 @@ class CronTrigger(BaseModel): @field_validator("schedule") @classmethod def validate_cron_schedule(cls, v: str) -> str: - return validate_cron_schedule_value(v) + schedule = validate_cron_schedule_value(v) + + # Import lazily to keep the schema/config dependency one-way at import time. + from openhands.automation.config import get_config + + floor = get_config().service.min_cron_interval_seconds + if floor > 0 and min_interval_seconds(schedule) < floor: + raise PydanticCustomError( + "interval_too_short", + "Cron expression must have at least {floor} seconds between fires", + {"floor": floor}, + ) + return schedule @field_validator("timezone") @classmethod diff --git a/tests/test_capabilities_router.py b/tests/test_capabilities_router.py index 605a3202..3fcb05c9 100644 --- a/tests/test_capabilities_router.py +++ b/tests/test_capabilities_router.py @@ -4,11 +4,14 @@ import uuid import pytest +from sqlalchemy.ext.asyncio import AsyncSession from openhands.automation.app import app from openhands.automation.auth import authenticate_request +from openhands.automation.capabilities_router import validate_draft from openhands.automation.config import clear_config_cache from openhands.automation.models import CustomWebhook +from openhands.automation.schemas import ValidateDraftRequest # Test UUID matching mock_authenticated_user fixture @@ -225,6 +228,17 @@ async def test_advertised_cron_floor_follows_the_scheduler_interval( assert response.json()["triggers"]["cron"]["minIntervalSeconds"] == 300 + async def test_advertises_a_higher_configured_cron_floor( + self, async_client, ready_deployment, monkeypatch + ): + """Clients see the same deployment floor enforced by trigger validation.""" + monkeypatch.setenv("AUTOMATION_MIN_CRON_INTERVAL_SECONDS", "900") + clear_config_cache() + + response = await async_client.get(CAPABILITIES_URL) + + assert response.json()["triggers"]["cron"]["minIntervalSeconds"] == 900 + class TestValidateDraft: """Tests for POST /v1/validate endpoint.""" @@ -288,6 +302,43 @@ async def test_schedule_faster_than_the_deployment_floor_is_rejected( assert body["valid"] is False assert addressed_errors(body) == [("trigger.schedule", "interval_too_short")] + @pytest.mark.parametrize( + ("endpoint", "draft"), + [("/v1/preset/prompt", CRON_DRAFT), ("/v1", BUNDLE_DRAFT)], + ) + @pytest.mark.parametrize( + ("floor", "schedule", "expected_errors"), + [ + (0, "*/2 * * * *", []), + (300, "*/2 * * * *", [("trigger.schedule", "interval_too_short")]), + (300, "*/5 * * * *", []), + ], + ) + async def test_configured_cron_floor_preserves_preflight_error_codes( + self, + mock_authenticated_user, + monkeypatch, + endpoint, + draft, + floor, + schedule, + expected_errors, + ): + monkeypatch.setenv("AUTOMATION_MIN_CRON_INTERVAL_SECONDS", str(floor)) + clear_config_cache() + + request = ValidateDraftRequest.model_validate( + preflight(with_trigger(draft, schedule=schedule), endpoint=endpoint) + ) + async with AsyncSession() as session: + response = await validate_draft( + request, user=mock_authenticated_user, session=session + ) + + body = response.model_dump() + assert body["valid"] == (expected_errors == []) + assert addressed_errors(body) == expected_errors + @pytest.mark.parametrize( ("event_pattern", "expected_errors"), [ diff --git a/tests/test_config.py b/tests/test_config.py index 657cc3d0..5975a2a5 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -72,6 +72,22 @@ def test_failure_disable_threshold_uses_documented_env_var(self, monkeypatch): assert settings.failure_disable_threshold == 0 +class TestCronIntervalSettings: + def test_default_disables_minimum_cron_interval(self, monkeypatch): + monkeypatch.delenv("AUTOMATION_MIN_CRON_INTERVAL_SECONDS", raising=False) + + assert ServiceSettings().min_cron_interval_seconds == 0 + + def test_loads_minimum_cron_interval_from_environment(self, monkeypatch): + monkeypatch.setenv("AUTOMATION_MIN_CRON_INTERVAL_SECONDS", "300") + + assert ServiceSettings().min_cron_interval_seconds == 300 + + def test_rejects_negative_minimum_cron_interval(self): + with pytest.raises(ValueError, match="greater than or equal to 0"): + ServiceSettings(min_cron_interval_seconds=-1) + + class TestBasePath: """Verify base_path is derived from base_url path + /api/automation.""" diff --git a/tests/test_schemas.py b/tests/test_schemas.py index 025d1cbd..1d7939f9 100644 --- a/tests/test_schemas.py +++ b/tests/test_schemas.py @@ -14,6 +14,7 @@ import pytest from pydantic import ValidationError +from openhands.automation.config import clear_config_cache from openhands.automation.schemas import ( AutomationResponse, AutomationRunResponse, @@ -54,6 +55,9 @@ def test_non_utc_aware_datetime_is_unchanged(self): class TestCronTriggerValidation: + def teardown_method(self): + clear_config_cache() + def test_accepts_valid_cron_and_timezone(self): trigger = CronTrigger(schedule="0 9 * * *", timezone="America/New_York") @@ -70,6 +74,28 @@ def test_rejects_invalid_timezone(self): with pytest.raises(ValidationError, match="Invalid timezone"): CronTrigger(schedule="0 9 * * *", timezone="Not/A_Timezone") + def test_default_allows_any_valid_interval(self): + trigger = CronTrigger(schedule="* * * * *") + + assert trigger.schedule == "* * * * *" + + def test_rejects_schedule_below_configured_interval(self, monkeypatch): + monkeypatch.setenv("AUTOMATION_MIN_CRON_INTERVAL_SECONDS", "300") + clear_config_cache() + + with pytest.raises(ValidationError, match="at least 300 seconds") as exc_info: + CronTrigger(schedule="* * * * *") + + assert exc_info.value.errors()[0]["type"] == "interval_too_short" + + def test_accepts_schedule_at_configured_interval(self, monkeypatch): + monkeypatch.setenv("AUTOMATION_MIN_CRON_INTERVAL_SECONDS", "300") + clear_config_cache() + + trigger = CronTrigger(schedule="*/5 * * * *") + + assert trigger.schedule == "*/5 * * * *" + class TestAutomationStateEnabledValidation: _CREATE_PAYLOAD: ClassVar[dict[str, Any]] = {