feat: enforce configurable minimum cron intervals

[Linxiushen]

Summary

• Add AUTOMATION_MIN_CRON_INTERVAL_SECONDS, with 0 preserving the existing disabled-by-default behavior.
• Reject cron triggers whose shortest sampled interval is below the configured floor.
• Advertise the effective floor as the maximum of polling, scheduler, and deployment limits.
• Preserve the interval_too_short preflight error code when the configured floor is enabled, while retaining schema enforcement for create/update requests.

Closes #217

Validation

• Frozen Linux dependencies, including OpenHands SDK 1.49.5 and LiteLLM 1.93.0.
• uv run --no-sync python -m pytest tests/test_schemas.py tests/test_config.py tests/test_capabilities_router.py::TestValidateDraft::test_configured_cron_floor_preserves_preflight_error_codes -q --tb=short: 83 passed.
• Before the error-code fix, the focused schema/preflight regression produced 3 failed and 9 passed. The failures were the schema error code and the prompt/bundle preflight error codes.
• Real FastAPI router requests through httpx.ASGITransport verified both /v1/preset/prompt and /v1 draft types: with floor 300, */2 * * * * returns HTTP 200 with valid: false and trigger.schedule / interval_too_short; floor 0 and the */5 * * * * boundary remain valid. The six checks had two contract mismatches before the fix and none after it.
• Full PRE_COMMIT_NO_CONCURRENCY=1 uv run pre-commit run --all-files --show-diff-on-failure: passed, including Pyright. Checks ran within a 1 CPU / 6 GiB container; serial execution avoids the default parallel Pyright processes exceeding that memory limit.

The cron validation path does not access the database. Its focused tests and HTTP proof use an unbound SQLAlchemy session; the complete PostgreSQL integration suite has not been rerun locally for this update. Hosted CI is pending for the new commit.

[all-hands-bot]

👋 This PR needs a couple of things fixed before OpenHands can review it:

• the PR description's HUMAN: section needs at least 20 characters describing what you tested, not just the template placeholder

Push an update once this is addressed and this check re-runs automatically.

This is an automated check - no AI was used to generate this comment.

[all-hands-bot]

This review was posted by an AI agent (OpenHands).

Summary

The change adds AUTOMATION_MIN_CRON_INTERVAL_SECONDS (default 0, disabled) and advertises max(POLL_INTERVAL_SECONDS, scheduler_interval_seconds, min_cron_interval_seconds) as the effective cron floor, while separately enforcing the floor inside CronTrigger.validate_cron_schedule. The intent matches linked issue #217 and all checks on this head are green (backend, unit-tests, Build and Push Automation Image, check-pr-artifacts, Validate PR description).

The material problem is that the enforcement path and the advertised/preflight floor are computed from different inputs, so they disagree whenever the knob is set below the scheduler interval, or left at the default 0 while the scheduler is slow. Focused tests pass locally (uv run pytest tests/test_schemas.py tests/test_config.py -q -> 77 passed). tests/test_capabilities_router.py cannot run here (needs Docker/testcontainers).

Findings

1. Create/update enforcement uses a smaller floor than the one advertised and used by preflight (openhands/automation/schemas.py:77)

The validator reads only min_cron_interval_seconds, but the effective deployment floor is the composed max(POLL_INTERVAL_SECONDS, scheduler_interval_seconds, min_cron_interval_seconds) (capabilities_router.py:249). With AUTOMATION_SCHEDULER_INTERVAL_SECONDS=300 and AUTOMATION_MIN_CRON_INTERVAL_SECONDS=120, I confirmed on this head that CreatePromptAutomationRequest(... schedule="*/2 * * * *") is accepted, while GET /v1/capabilities advertises minIntervalSeconds == 300 and _cron_errors returns interval_too_short for the same schedule. A schedule the deployment says it cannot honour is creatable through the real endpoint.

2. With the default knob the authoritative path enforces nothing while capabilities advertises a floor (openhands/automation/capabilities_router.py:252)

Adding settings.min_cron_interval_seconds to the advertised max(...) without routing enforcement through the same composed value leaves the create/update path unenforced at the default 0, yet capabilities still reports max(POLL_INTERVAL_SECONDS, scheduler_interval_seconds). Confirmed: with AUTOMATION_SCHEDULER_INTERVAL_SECONDS=300 and the knob unset, */2 * * * * is accepted while the advertised floor is 300. This widens the pre-existing create-vs-preflight gap rather than closing it.

3. Raising inside the field validator changes the /v1/validate error code once the knob is set (openhands/automation/schemas.py:78)

Because the check now raises during Pydantic model validation, validate_draft returns _schema_errors(e) before _cron_errors runs. On this head, with the knob at 300, a */2 * * * * draft yields ("trigger.schedule", "value_error") instead of the contract's interval_too_short (asserted by tests/test_capabilities_router.py:300 and :425); the structured-code branch at capabilities_router.py:264 becomes unreachable whenever the knob is enabled. Enforcing only in _cron_errors (or sharing one floor helper) would preserve the contract.

Notes

• Consolidating findings 1-3 on a single composed-floor helper removes all of the divergence at once.
• Side effect worth confirming as intended: on current main, CronTrigger is also deserialized from stored automation.yaml via TypeAdapter(Trigger) in git_sync/loop.py. Raising the enforcement floor makes re-reading an already-persisted fast schedule raise ValidationError; the git-sync import loop catches (ValidationError, ValueError) and skips the directory, so a previously valid automation can silently stop importing after this ships.
• The branch does not merge cleanly as-is (mergeable_state: behind); a rebase is advisable before merge.

The PR description's claim that the floor is advertised as the maximum of polling, scheduler, and deployment limits is correct for /capabilities; the equivalent claim does not hold for trigger validation.

🔄 CHANGES REQUESTED

[all-hands-bot]

The enforcement floor here is only min_cron_interval_seconds, but the effective deployment floor advertised by /capabilities and used by /v1/validate is max(POLL_INTERVAL_SECONDS, scheduler_interval_seconds, min_cron_interval_seconds) (capabilities_router.py:249). With AUTOMATION_SCHEDULER_INTERVAL_SECONDS=300 and AUTOMATION_MIN_CRON_INTERVAL_SECONDS=120, CreatePromptAutomationRequest/UpdateAutomationRequest accept */2 * * * * while capabilities reports a 300s floor and _cron_errors rejects the same schedule. Please enforce the same composed value here (or share one helper) so the authoritative path cannot accept a schedule the deployment advertises as un-honourable.

[all-hands-bot]

Including min_cron_interval_seconds in the advertised max(...) only matches the create/update enforcement when this knob is the largest term. At the default 0 the schema validator performs no floor check at all, yet capabilities still advertises max(POLL_INTERVAL_SECONDS, scheduler_interval_seconds). Confirmed on this head: with AUTOMATION_SCHEDULER_INTERVAL_SECONDS=300 and the knob unset, */2 * * * * is accepted at create time while the advertised floor is 300. Routing enforcement through this same helper would close the gap this PR is meant to close.

[all-hands-bot]

Raising ValueError inside the field validator turns the floor violation into a Pydantic value_error, so validate_draft returns _schema_errors(e) before _cron_errors can run. On this head, with the knob at 300, a */2 * * * * draft yields ("trigger.schedule", "value_error") rather than the documented interval_too_short (asserted in tests/test_capabilities_router.py:300 and :425), making the structured-code branch at capabilities_router.py:264 unreachable whenever the knob is enabled. Enforcing only in _cron_errors would keep the existing error contract for clients keyed on the code.

[Linxiushen]

Addressed in 40fbfac. Fixed with PydanticCustomError("interval_too_short", ...), so _schema_errors preserves the preflight code for both prompt and bundle drafts. The validator remains in place because #217 requires create/update requests to enforce the configured minimum as well. The focused regression fails on the previous head and passes after this change; real HTTP preflight checks confirm the code with the knob set to 300, while floor 0 and the five-minute boundary still pass.

[all-hands-bot]

This review was posted by an AI agent (OpenHands).

Summary

Reviewed head 40fbfac after the author's follow-up. My previous finding #3 (the /v1/validate error code collapsing to value_error) is fixed: the validator now raises PydanticCustomError("interval_too_short", ...), and I confirmed on this head that a */2 * * * * draft yields ("trigger.schedule", "interval_too_short") through the schema path. All checks on this head are green (backend, unit-tests, Build and Push Automation Image, Validate PR description); uv run --no-sync pytest tests/test_schemas.py tests/test_config.py -q -> 77 passed locally.

However, the core divergence from my earlier findings #1 and #2 is still present: the create/update enforcement and the advertised/preflight floor are computed from different inputs, so they disagree whenever the knob is below the scheduler interval, or when the knob is left at the default 0 while the scheduler is slow.

Findings

1. Create/update enforcement reads only the new knob, not the composed floor (openhands/automation/schemas.py:78)

The validator uses min_cron_interval_seconds alone, while the effective deployment floor advertised by /capabilities and enforced by _cron_errors is max(POLL_INTERVAL_SECONDS, scheduler_interval_seconds, min_cron_interval_seconds) (capabilities_router.py:249). Confirmed on this head with AUTOMATION_SCHEDULER_INTERVAL_SECONDS=300 and AUTOMATION_MIN_CRON_INTERVAL_SECONDS=120: CreatePromptAutomationRequest(... schedule="*/2 * * * *") is accepted, while _cron_interval_floor() returns 300 and _cron_errors returns interval_too_short for the identical schedule. A schedule the deployment advertises as un-honourable is still creatable.

2. At the default knob the authoritative path enforces nothing while capabilities advertises a floor (openhands/automation/capabilities_router.py:252)

The advertised max(...) now includes the new knob, but enforcement still only triggers when that knob is > 0. Confirmed on this head with AUTOMATION_SCHEDULER_INTERVAL_SECONDS=300 and the knob unset: */2 * * * * is accepted at create time while _cron_interval_floor() returns 300. So in the shipped default configuration the authoritative create/update path remains unenforced while the UI is told a 300s floor applies.

Both findings have the same fix: route the schema validator through _cron_interval_floor() (or a shared helper) instead of reading the raw setting, so creation, preflight, and capabilities all agree.

Notes

• Side effect worth confirming as intended: CronTrigger is also deserialized from stored automation.yaml via TypeAdapter(Trigger) in git_sync/loop.py. I confirmed on this head that raising the knob to 300 makes re-reading an existing */2 * * * * schedule raise ValidationError, and the git-sync import loop catches (ValidationError, ValueError) at git_sync/loop.py:777 and skips that directory — so a previously valid automation can silently stop importing after this ships.
• The PR description is accurate that the error code is preserved; the separate claim that the advertised floor equals the enforcement floor does not hold in the two configurations above.
• The branch is mergeable_state: behind; a rebase before merge is advisable.

🔄 CHANGES REQUESTED

[all-hands-bot]

This reads only min_cron_interval_seconds, but the effective deployment floor used by /capabilities and _cron_errors is max(POLL_INTERVAL_SECONDS, scheduler_interval_seconds, min_cron_interval_seconds) (capabilities_router.py:249). Confirmed on this head with AUTOMATION_SCHEDULER_INTERVAL_SECONDS=300 and AUTOMATION_MIN_CRON_INTERVAL_SECONDS=120: CreatePromptAutomationRequest accepts */2 * * * * while the advertised floor is 300 and _cron_errors rejects the same schedule. Please enforce the composed floor here (or share a helper with _cron_interval_floor) so create/update cannot accept a schedule the deployment advertises as un-honourable.

[all-hands-bot]

Adding min_cron_interval_seconds to the advertised max(...) does not line the advertisement up with enforcement, because the schema validator still only checks that knob when it is > 0. Confirmed on this head with AUTOMATION_SCHEDULER_INTERVAL_SECONDS=300 and the knob unset: */2 * * * * is accepted at create time while _cron_interval_floor() returns 300. In the default configuration the authoritative path enforces nothing while clients are told a 300s floor applies. Routing enforcement through this same helper would close the gap this PR targets.

[Linxiushen]

The remaining findings point to a contract choice that conflicts with #217's explicit acceptance criteria: "default 0 = no floor (preserves current behavior)" and "accept anything when floor=0". The schema currently implements that opt-in policy.

I checked the PR base (2644218): CronTrigger only validates cron syntax, while _cron_interval_floor() already takes max(POLL_INTERVAL_SECONDS, scheduler_interval_seconds) for capabilities/preflight. Thus the slow-scheduler discrepancy at floor=0 predates this PR. Adding the knob to that maximum preserves the existing operational preflight limit; it does not make it the schema's opt-in policy.

Unconditionally using the composed floor in CronTrigger would reject previously accepted create/update requests and git-sync definitions even when the new knob is left at 0. Could a maintainer confirm whether #217's compatibility requirement should be superseded by that behavior? If so, the change needs explicit coverage for existing stored schedules as well as the request paths. Otherwise, I propose retaining the requested opt-in enforcement and treating the pre-existing scheduler/preflight discrepancy separately.

The error-code correction remains covered by the previously reported 83 tests and six HTTP preflight checks; I am not claiming those cover a changed default enforcement policy.

[enyst]

I'm an AI agent (Claude, based on Opus 5.5) helping Engel Nyst (@enyst) with project work.

No blocking defects found; I'm leaving the approval to a maintainer because the open question below is a policy choice. The current head (40fbfac) does what #217 asks for: an opt-in floor that is off at 0, enforced on create and update, with the interval_too_short preflight code kept. CI is green on this head, and it merges cleanly with current main.

On the open bot findings: the value_error code finding is fixed. On the remaining gap between what create/update enforces and what /capabilities advertises, I agree with @Linxiushen. At the base (2644218), CronTrigger validated only syntax, while _cron_interval_floor() already returned max(POLL_INTERVAL_SECONDS, scheduler_interval_seconds). So the gap at the default 0 predates this PR, and #217 explicitly requires that 0 accept anything. There is an optional, compatible way to close the knob-below-scheduler case: when the knob is > 0, enforce max(knob, POLL_INTERVAL_SECONDS, scheduler_interval_seconds) through a shared helper. That keeps 0 as "no floor" and still satisfies #217's examples.

On the git-sync note: a directory below the floor is skipped whenever the import re-parses it. I checked that a skip never deletes the automation. On main, soft-delete only happens when the directory is missing from all_dirs, not when parsing fails.

Scheduling notes:

• The floor only applies when a trigger is validated. scheduler.py evaluates the stored automation.trigger dict directly, so automations already below a newly raised floor keep firing at their original rate until someone edits them.
• A floor does not stop an automation from overlapping itself. max_run_duration defaults to 30 minutes, and the watchdog allows provisioning time and grace on top of that. Any floor below that still lets a long run overlap the next fire, so operators should pair this with the overlap guard (#218/#283) rather than treat it as one.

What I verified: the diff; CronTrigger and _cron_interval_floor at the base vs. the head; a clean git merge-tree against current main, including #439's draft path (it validates drafts with the separate CronTriggerDraft and checks the floor via the composed _cron_errors); and the green checks on 40fbfac. I did not run the tests locally.