From 337d33afeb9fb5d4297ce1885add70e63e5f88b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9E=97SO?= <142557582+Linxiushen@users.noreply.github.com> Date: Wed, 5 Aug 2026 09:23:31 +0800 Subject: [PATCH] feat: enforce configurable minimum cron intervals --- openhands/automation/capabilities_router.py | 7 +++++- openhands/automation/config.py | 5 ++++- openhands/automation/schemas.py | 13 ++++++++++- tests/test_capabilities_router.py | 11 ++++++++++ tests/test_config.py | 19 ++++++++++++++++ tests/test_schemas.py | 24 +++++++++++++++++++++ 6 files changed, 76 insertions(+), 3 deletions(-) diff --git a/openhands/automation/capabilities_router.py b/openhands/automation/capabilities_router.py index f03f3915..380dc078 100644 --- a/openhands/automation/capabilities_router.py +++ b/openhands/automation/capabilities_router.py @@ -233,7 +233,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 0b8727da..455363ef 100644 --- a/openhands/automation/config.py +++ b/openhands/automation/config.py @@ -48,7 +48,7 @@ from typing import Literal from urllib.parse import urlparse -from pydantic import model_validator +from pydantic import Field, model_validator from pydantic_settings import BaseSettings @@ -327,6 +327,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) @@ -412,6 +414,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 4045d19d..a76d77ba 100644 --- a/openhands/automation/schemas.py +++ b/openhands/automation/schemas.py @@ -10,6 +10,7 @@ from openhands.automation.constants import MODEL_PROFILE_PATTERN from openhands.automation.utils.cron import ( + min_interval_seconds, validate_cron_schedule as validate_cron_schedule_value, validate_timezone_name, ) @@ -42,7 +43,17 @@ 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 ValueError( + f"Cron expression must have at least {floor} seconds between fires" + ) + return schedule @field_validator("timezone") @classmethod diff --git a/tests/test_capabilities_router.py b/tests/test_capabilities_router.py index 60b57bcb..dd344b15 100644 --- a/tests/test_capabilities_router.py +++ b/tests/test_capabilities_router.py @@ -217,6 +217,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.""" diff --git a/tests/test_config.py b/tests/test_config.py index 1bf78a98..b2eea9b1 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -2,10 +2,13 @@ import warnings +import pytest + from openhands.automation.config import ( HttpSettings, LogSettings, SandboxSettings, + ServiceSettings, Settings, clear_config_cache, get_config, @@ -57,6 +60,22 @@ def test_resolve_caps_stored_timeout_to_configured_max(self): assert resolve_automation_timeout_seconds(max_duration + 600) == max_duration +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 cbe74073..bbd410ed 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, @@ -49,6 +50,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") @@ -65,6 +69,26 @@ 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"): + CronTrigger(schedule="* * * * *") + + 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 TestAutomationRunResponseUtcSerialisation: """AutomationRunResponse must include a UTC offset in all datetime fields."""