fix: auto-disable unhealthy automations

[malhotra5]

Summary

Implements Linear OSS-9472 by auto-disabling automations that repeatedly fail for permanent, user-actionable reasons and exposing the current blocking reason on the automation API response.

Fixes #323

Changes include:

• classify permanent automation failures from structured status_detail / SDK callback metadata (auth, config, quota, blocked, user_action=settings, non-retryable blocking factors)
• auto-disable after a configurable consecutive permanent failure threshold (AUTOMATION_FAILURE_DISABLE_THRESHOLD, default 3; <=0 disables the behavior)
• persist current disabled state on automations (disabled_reason, disabled_detail, disabled_at) for list/detail API consumers
• add automation_disable_events history table to track each auto-disable occurrence over time
• reject manual dispatch of disabled automations with 409 plus the blocking reason/detail
• skip dispatcher polling for pending runs whose automation is disabled/deleted
• preserve callback-reported blocking factors/task outcomes as run.status_detail

Validation

Passed:

• uv run ruff check openhands/automation/models.py openhands/automation/config.py openhands/automation/dispatcher.py openhands/automation/router.py openhands/automation/schemas.py openhands/automation/utils/run.py openhands/automation/utils/run_status_detail.py openhands/automation/utils/unhealthy.py tests/test_unhealthy_automations.py tests/test_schemas.py tests/test_run_status_detail.py tests/test_dispatcher.py tests/test_router.py migrations/versions/017_add_automation_disabled_reason.py
• uv run pyright openhands/automation/models.py openhands/automation/utils/run.py openhands/automation/utils/unhealthy.py tests/test_unhealthy_automations.py
• uv run pytest tests/test_unhealthy_automations.py tests/test_run_status_detail.py tests/test_schemas.py -q

Not run successfully in this environment:

• targeted dispatcher/router tests that use the shared Postgres testcontainer fixture, because Docker is unavailable (/var/run/docker.sock missing).

Notes

No PR template file was present in .github/, so this uses the repo's conventional Summary/Validation format.

This PR was created by an AI agent (OpenHands) on behalf of the user.

@malhotra5 can click here to continue refining the PR

[github-actions]

[all-hands-bot]

🔍 Review in progress…

We are performing the review through OpenHands Cloud Automation. You can log in and view the conversation here.

[all-hands-bot]

🤖 OpenHands is reviewing this PR.

Head commit: 0f9c38f4a83449554d2d85b5bdf3bf9c691145f3
View the conversation: https://oss-agent-canvas.ngrok.dev/conversations/859cd840-c79b-4b62-b444-b657517ac0f7

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

[all-hands-bot]

This review was created by an AI agent (OpenHands) on behalf of the repository maintainers.

Summary

This PR implements auto-disable for chronically failing automations — classifying permanent failures from structured status_detail/SDK callback metadata, disabling after a configurable threshold, persisting disabled state, and blocking dispatch of disabled automations. The design is well-structured: optimistic locking, a history table, cross-database migration, and good test coverage.

Risk Assessment: Medium

The core auto-disable logic is sound and well-tested. However, there is one behavioral regression that can strand runs indefinitely, and one classification default that may be too aggressive.

Findings

1. Stranded PENDING runs on manual disable/delete (bug)

The dispatcher now filters out PENDING runs whose automation is enabled=False or deleted_at IS NOT NULL (dispatcher.py lines 127–128). The auto-disable paths (disable_automation and maybe_disable_unhealthy_automation) correctly call skip_pending_runs_for_disabled_automation to mark those runs SKIPPED. However, the manual paths do not:

• update_automation (PATCH enabled=false): Sets enabled=False but never calls skip_pending_runs_for_disabled_automation. PENDING runs created by the scheduler before the PATCH will never be dispatched (filtered out) and never marked terminal. They have no timeout_at (only set on RUNNING), so the watchdog won’t catch them either. They are stranded in PENDING indefinitely.

• delete_automation: Same issue — sets enabled=False + deleted_at, but PENDING runs are not skipped. Before this PR the dispatcher would still dispatch them; now they are silently dropped.

Suggestion: Call skip_pending_runs_for_disabled_automation in update_automation when enabled is set to False, and in delete_automation after soft-deleting. Alternatively, have the dispatcher mark filtered PENDING runs as SKIPPED rather than just ignoring them.

2. Aggressive default kind for failed task outcomes (design concern)

In blocking_factor_from_task_outcome, a task_outcome with success: false and no explicit kind defaults to RunStatusDetailKind.BLOCKED (line 209). Since "blocked" is in PERMANENT_FAILURE_KINDS, any failed task outcome without explicit retryable/classification metadata counts as a permanent failure. After threshold (default 3) such callbacks, the automation is auto-disabled.

This could be surprising for genuinely transient failures if the SDK reports them as success: false without a retryable flag. Consider defaulting to a non-permanent kind (e.g., UNKNOWN or EXECUTION_ERROR) and only treating it as permanent when explicit classification metadata is present.

Non-blocking notes

• maybe_disable_unhealthy_automation is called on every complete_run callback (including successful ones). The early-return when the latest run isn’t permanent makes this cheap (one indexed query returning ≤ threshold rows), so this is acceptable.
• The migration and model changes are clean and follow the cross-database conventions (generic SQLAlchemy types, SQLite conditionals for Postgres-only comments).

[all-hands-bot]

✅ Review complete.

This review was performed through OpenHands Cloud Automation. You can log in and view the conversation here.

[all-hands-bot]

🟡 Taste Rating: Acceptable - The data model and thresholding approach are straightforward, but one callback path drops the new structured metadata and breaks the core feature for failed runs.

[CRITICAL ISSUES]

• [openhands/automation/router.py, Line 571] Correctness: FAILED completion callbacks ignore blocking_factor / task_outcome, so permanent SDK-reported blocks on failed runs are not preserved in status_detail and will not count toward auto-disable.

[TESTING GAPS]

• Add a callback regression test for status: "FAILED" plus blocking_factor or task_outcome proving the resulting status_detail contains the permanent classification and that the unhealthy automation threshold can disable from that path.

[RISK ASSESSMENT]

• [Overall PR] ⚠️ Risk Assessment: 🟡 MEDIUM
  This changes run lifecycle behavior, persisted API response fields, and automatic disabling decisions. The migration and manual disable paths look reasonable, but missing failed-callback classification coverage means a primary production failure mode may silently bypass the new protection.

VERDICT:
❌ Needs rework: Preserve structured blocking metadata for failed callbacks before merging.

KEY INSIGHT:
The new state model is sound, but the callback normalization must be status-agnostic or failed runs lose the metadata the auto-disable classifier depends on.

Improve this review? If any feedback above seems incorrect or irrelevant to this repository, you can teach the reviewer to do better:

1. Add a .agents/skills/custom-codereview-guide.md file to your branch (or edit it if one already exists) with the /codereview trigger and the context the reviewer is missing (e.g., "Security concerns about X do not apply here because Y"). See the customization docs for the required frontmatter format.
2. Re-request a review - the reviewer reads guidelines from the PR branch, so your changes take effect immediately.
3. When your PR is merged, the guideline file goes through normal code review by repository maintainers.

Resolve with AI? Install the iterate skill in your agent and run /iterate to automatically drive this PR through CI, review, and QA until it is merge-ready.

Was this review helpful? React with 👍 or 👎 to give feedback.

This review was generated by an AI agent (OpenHands) on behalf of the user through OpenHands Automation. View conversation

stale

[all-hands-bot]

✅ Review complete.

This review was performed through OpenHands Cloud Automation. You can log in and view the conversation here.

[all-hands-bot]

🟡 Acceptable — Clean, well-motivated implementation of auto-disable for unhealthy automations. The core architecture is sound: conservative PERMANENT_FAILURE_KINDS classification, a configurable threshold, an audit event table, and correct pending-run cleanup on disable. Two minor efficiency issues and two testing gaps worth noting below; none are blocking.

[IMPROVEMENT OPPORTUNITIES]

See inline comments on specific lines.

[TESTING GAPS]

• complete_run → auto-disable integration path untested: test_unhealthy_automations.py tests maybe_disable_unhealthy_automation directly with pre-seeded runs, and test_router.py verifies that failed callbacks set the correct status_detail. But there is no end-to-end test that POSTs N failed callbacks via HTTP and asserts the automation ends up disabled. If the maybe_disable_unhealthy_automation(session, automation.id) call were accidentally removed from complete_run, nothing in the suite would catch it.

• Re-enable path untested: update_automation clears disabled_reason, disabled_detail, and disabled_at when enabled=True is patched. The disable direction is tested (test_update_automation_disable), but the reverse — re-enabling an auto-disabled automation and confirming the metadata is cleared — is not.

[RISK ASSESSMENT]

• [Overall PR] ⚠️ Risk Assessment: 🟡 MEDIUM
  Auto-disabling automations is operationally sensitive: a classification bug or an overly aggressive threshold could silently stop working automations. The 3-failure default, the conservative PERMANENT_FAILURE_KINDS set (auth/config/quota/blocked), and the idempotent re-enable path all reduce this risk materially. The main residual risk is that the threshold-based path in complete_run is thin and untested end-to-end.

VERDICT:
✅ Worth merging — core logic is correct and well-tested for primary paths; follow-up tickets for the two testing gaps would close the residual risk.

KEY INSIGHT:
The status_detail = None on COMPLETED runs is the right idiomatic mechanism for breaking the failure streak — simple and clean, the classifier never needs to reason about outcomes directly.

This review was generated by an AI agent (OpenHands) on behalf of the user through OpenHands Automation. View conversation

---

Improve this review? If any feedback above seems incorrect or irrelevant to this repository, you can teach the reviewer to do better:

1. Add a .agents/skills/custom-codereview-guide.md file to your branch (or edit it if one already exists) with the /codereview trigger and the context the reviewer is missing. See the customization docs for the required frontmatter format.
2. Re-request a review — the reviewer reads guidelines from the PR branch, so your changes take effect immediately.
3. When your PR is merged, the guideline file goes through normal code review by repository maintainers.

Resolve with AI? Install the iterate skill in your agent and run /iterate to automatically drive this PR through CI, review, and QA until it's merge-ready.

Was this review helpful? React with 👍 or 👎 to give feedback.