feat: auto-disable for consecutively failing automations [PLTF-3374]

[dylan-openhands]

Human Explanation

HUMAN: Broken automations run forever and recently we found that ~30 automations that have never succeeded account for almost 3,000 sandbox creations per day in SaaS.

This PR adds a default-off rule to disable automations that have failed X times in a row over Y window. I intend to configure those values in SaaS for 10 times in a 24 hour window

Tested locally: full unit suite passes (1442). New coverage asserts the rule is off unless the env var is set

AI Description

Off by default. One env var turns it on:

Env var	Default	Effect
AUTOMATION_CONSECUTIVE_FAILURE_DISABLE_THRESHOLD	unset	Unset = rule off. Set a number = rule on at that number.
AUTOMATION_CONSECUTIVE_FAILURE_DISABLE_WINDOW_HOURS	24	Only fires if nothing succeeded in this many hours.

The window is the outage guard. A run count alone can't tell "broken forever" from "GitHub was down" — a 5-minute cron racks up ~96 failures in an 8h outage. Asking "has anything succeeded lately?" answers that directly and is indifferent to how often the automation runs. Set it to 48 or 72 to ride out longer outages.

Scenario	last N all failed	success in window	disables
5-min cron, broken a week	yes	no	yes
5-min cron, 8h outage	yes	yes	no
Daily cron, 10 failures	yes	no	yes
Flaky, succeeds 1-in-20	no	yes	no

Known gap: a flapping automation that succeeds occasionally never fires, since LIMIT N won't come back all-failed. A failure-rate rule would catch it; it needed an unbounded count/sum aggregate over each automation's full history and a new composite index to stay fast, which wasn't worth a migration for the secondary case. Deliberately left out.

Surfacing

Reuses what's already on AutomationResponse. disabled_reason is now a plain sentence — "Paused automatically: the last 10 runs all failed and nothing has succeeded in 24 hours." — and disabled_detail.rule tells the UI which rule fired. Queued runs already get SKIPPED with a reason, and the automation_disable_events audit row is unchanged.

Files

• utils/unhealthy.py — both rules now evaluate from one fetch of recent terminal runs. Counting is scoped to runs after the last disable event, so re-enabling doesn't instantly re-disable on stale history.
• config.py — the two knobs above.
• dispatcher.py — drops the status_detail is not None guard (MVP: Automation service with CRUD API, cron scheduler, and V1 API executor #2).
• watchdog.py — evaluates the rules after a stale run goes terminal (Migrate automation source code from deploy repo #3).

Note that #2 and #3 are not behind the flag. With the threshold unset they only mean the pre-existing permanent rule is evaluated where it always should have been.

Deploying

Ship the tag with the env var absent, confirm it's inert, then we will turn it on as a separate chart change

• An unknown env var on an older image is tolerated (no extra="forbid"), so setting it early is harmless.
• An empty value is not. AUTOMATION_CONSECUTIVE_FAILURE_DISABLE_THRESHOLD="" raises ValidationError and the pod won't boot. Guard the key, not the value:

{{- if .Values.autoDisable.consecutiveFailures }}
- name: AUTOMATION_CONSECUTIVE_FAILURE_DISABLE_THRESHOLD
  value: {{ .Values.autoDisable.consecutiveFailures | quote }}
{{- end }}

0 also parses and means off; "" never does.

Verification

Full suite: 1442 passed, 49 skipped. 16 tests in test_unhealthy_automations.py, including two that pin the behavior that motivated the design:

• test_high_frequency_automation_broken_for_days_is_disabled — a 5-min cron with 200 straight failures spans under an hour across its last 10 runs. An earlier span-based version of this rule let it run forever; this catches it.
• test_a_longer_window_rides_out_a_longer_outage — the same 30h outage disables at 24 and declines at 72.

Plus: off-by-default, opt-in via the env var, outage burst declines, a recent success breaks the streak, and re-enable clears prior history.

Local note: testcontainers can't start on Colima (ryuk bind-mounts the docker socket, virtiofs won't create a socket node). TESTCONTAINERS_RYUK_DISABLED=true works around it; containers still clean up via conftest's context manager. CI is unaffected.

[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.

[github-actions]

[aivong-openhands]

Mutation review of the new tests

I hand-wrote a small set of mutants against the diff and ran them with the suite (tests/test_unhealthy_automations.py, baseline 16 passed in ~1.5s; wiring mutants against the full 1491-passed suite). This looks at test strength only — where a deliberate behaviour change survives green.

Controls — these died, so the core is genuinely pinned

Mutant	Result
Never fire the consecutive rule (cause = None)	❌ caught
Force the rule on when the threshold is unset	❌ caught
Success-window lookup inverted (created_at < since)	❌ caught
Success window widened 100×	❌ caught
Off-by-one on the streak (>= threshold → > threshold)	❌ caught
Success filter treats non-COMPLETED as success	❌ caught

What makes these land: the tests assert the boolean outcome and disabled_detail["rule"]/consecutive_failures/the reason string, and they drive both sides of the window boundary (test_outage_burst_does_not_disable, test_a_longer_window_rides_out_a_longer_outage). The "on/off/off-by-one" trio is solid.

Survivors (gaps)

Mutant	Result
Watchdog timeout no longer evaluates auto-disable (#3)	✅ survived (full suite)
Drop the re-enable boundary clamp on the success window	✅ survived
_humanize days branch drops the /24	✅ survived

---

1. The watchdog wiring (#3) is unasserted — and it's the motivating case

mark_stale_runs has no test anywhere in the suite, so deleting the new call it makes leaves everything green:

if terminal:
    marked += 1
    pass  # await maybe_disable_unhealthy_automation_after_run(...) removed — suite still passes

This is exactly the "an automation that only ever times out ran forever" scenario the PR is meant to fix: the run is marked FAILED by the watchdog, but nothing re-checks whether the automation should now be disabled. A regression here is invisible.

A test that closes it (drives a stale RUNNING run to a watchdog-authored FAILED as the Nth failure and asserts the automation is disabled):

@pytest.mark.asyncio
async def test_watchdog_timeout_disables_a_chronically_failing_automation(
    async_session_factory, mock_settings, monkeypatch
):
    from openhands.automation.config import clear_config_cache
    from openhands.automation.utils.agent_server import VerificationResult
    from openhands.automation.watchdog import mark_stale_runs

    monkeypatch.setenv("AUTOMATION_CONSECUTIVE_FAILURE_DISABLE_THRESHOLD", "10")
    monkeypatch.setenv("AUTOMATION_CONSECUTIVE_FAILURE_DISABLE_WINDOW_HOURS", "24")
    clear_config_cache()

    now = utcnow()
    async with async_session_factory() as session:
        automation = Automation(
            user_id=TEST_USER_ID, org_id=TEST_ORG_ID, name="Only ever times out",
            trigger={"type": "cron", "schedule": "* * * * *", "timezone": "UTC"},
            tarball_path="s3://bucket/code.tar.gz", entrypoint="uv run main.py",
            enabled=True, timeout=60,
        )
        session.add(automation)
        await session.flush()
        for i in range(9):  # 9 prior failures in-window
            session.add(AutomationRun(
                automation_id=automation.id, status=AutomationRunStatus.FAILED,
                created_at=now - timedelta(hours=i + 1),
                completed_at=now - timedelta(hours=i + 1),
            ))
        session.add(AutomationRun(  # 10th run: stale RUNNING, watchdog marks it FAILED
            automation_id=automation.id, status=AutomationRunStatus.RUNNING,
            sandbox_id="sb-timeout",
            started_at=now - timedelta(minutes=5),
            timeout_at=now - timedelta(minutes=1),
        ))
        await session.commit()
        automation_id = automation.id

    mock_backend = _create_mock_backend(
        VerificationResult(verified=False, error="Sandbox not available")
    )
    with patch("openhands.automation.watchdog.get_backend", return_value=mock_backend):
        marked = await mark_stale_runs(async_session_factory, mock_settings)

    assert marked == 1
    async with async_session_factory() as session:
        automation = await session.get(Automation, automation_id)
        assert automation.enabled is False
        assert automation.disabled_detail["rule"] == "consecutive_failures"
    clear_config_cache()

Verified: this passes on the branch as-is, and fails when the maybe_disable_unhealthy_automation_after_run(...) call in mark_stale_runs is removed.

---

2. The re-enable boundary clamp on the success window is unasserted

In maybe_disable_unhealthy_automation:

since = utcnow() - timedelta(hours=window_hours)
if boundary is not None:
    since = max(since, ensure_utc(boundary))   # <-- deleting this line survives

test_re_enabling_clears_prior_failure_history only adds a single failure after re-enable, so the success-lookup block never runs — the clamp on since is never exercised. Dropping it means the success query falls back to the full window_hours, so a success from before the last pause, still inside the window, suppresses a fresh all-failed streak. Concretely: fix an automation, re-enable it, it breaks again and fails N times straight — but because it succeeded once shortly before you paused it (and that's still inside 24h), it never gets re-disabled. That's the "runs forever" failure the rule exists to stop, reappearing right after a manual re-enable.

async def test_success_before_re_enable_does_not_block_re_disable(
    sqlite_session_factory, auto_disable_config
):
    auto_disable_config()  # threshold 10, window 24h
    async with sqlite_session_factory() as session:
        automation = await _create_automation(session)
        boundary = utcnow() - timedelta(hours=2)
        session.add(AutomationDisableEvent(
            automation_id=automation.id, run_id=None, reason="previous pause",
            detail={}, source="consecutive_failures", created_at=boundary,
        ))
        await session.flush()
        # Success from *before* the pause, still inside the 24h window.
        await _add_run_at(
            session, automation,
            status=AutomationRunStatus.COMPLETED, at=utcnow() - timedelta(hours=3),
        )
        # 10 fresh failures after re-enable (after the boundary).
        await _add_failures(session, automation, count=10, every=timedelta(minutes=10))

        assert await maybe_disable_unhealthy_automation(session, automation.id) is True

Verified: passes on the branch, fails when since = max(since, ensure_utc(boundary)) is dropped.

---

3. _humanize days branch (low severity, cosmetic)

def _humanize(hours: float) -> str:
    if hours < 48:
        return f"{round(hours)} hours"
    return f"{round(hours / 24)} days"   # dropping /24 survives

Every test uses the 24h window, so the >= 48h branch is never checked. With a 72h window the disabled_reason would read "72 days" instead of "3 days" and no test would notice. Only affects the human-readable reason string, so it's minor — one assertion closes it:

async def test_window_in_days_is_humanized(sqlite_session_factory, auto_disable_config):
    auto_disable_config(window_hours="72")
    async with sqlite_session_factory() as session:
        automation = await _create_automation(session)
        await _add_failures(session, automation, count=10, every=timedelta(hours=1))
        assert await maybe_disable_unhealthy_automation(session, automation.id) is True
        await session.refresh(automation)
        assert "3 days" in automation.disabled_reason  # verified: fails without /24

---

Not a test gap

• Dispatcher change (MVP: Automation service with CRUD API, cron scheduler, and V1 API executor #2) is an equivalent mutant, not a coverage gap. Reverting else: back to elif status_detail is not None: survives the whole suite — but that's because every _fail(...) call site already passes a non-None dict (make_run_status_detail / run_status_detail_from_exception both return dict[str, Any]), so the guard can never be False today. The change is harmless future-proofing with no observable behaviour to assert. No test needed.
• _leading_failures / _check_permanent "consecutive" counting is equivalent under the shipped config. Changing the break to continue in either loop survives, because limit == max(threshold, consecutive) — to reach the count you need every fetched run to be FAILED/permanent, so "leading" and "total among the window" coincide. It stops being equivalent only if the permanent threshold is ever configured above the consecutive threshold (then the window is wider than the count it feeds _check_permanent). Worth a one-line comment noting the counters rely on limit equalling the threshold; not worth a test today.

---

This comment was generated by an AI assistant on behalf of the user.

[all-hands-bot]

🤖 OpenHands is reviewing this PR.

Head commit: b53848e58b6a795da0e10a53100c64cd2967e4bf
View the conversation: https://oss-agent-canvas.ngrok.dev/conversations/b386d157-3eb1-46dd-9e86-2067704d13e4

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

[all-hands-bot]

🤖 OpenHands is reviewing this PR.

Head commit: b53848e58b6a795da0e10a53100c64cd2967e4bf
View the conversation: https://oss-agent-canvas.ngrok.dev/conversations/0c494541-9c78-4a08-b8b8-3771c7105016

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

[all-hands-bot]

⚠️ OpenHands PR Reviewer encountered a problem at commit b53848e58b6a (status: error).

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

[all-hands-bot]

⚠️ OpenHands PR Reviewer encountered a problem at commit b53848e58b6a (status: error).

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