feat(crons): Always create detectors for all monitors in MonitorValidator

[evanpurkhiser]

Previously, detectors were created as part of the endpoint, after detector → alerts linking happened. This changes it so detectors are crated immediately after the cron monitor is created.

Changes:

• Moved ensure_cron_detector() call from endpoint into MonitorValidator.create()
• Detector is created BEFORE alert_rule creation, allowing IssueAlertMigrator
  to find and link it to workflows
• Added skip_detector_creation context flag to prevent duplicate creation
  when using MonitorDataSourceValidator (new detector flow)
• Updated tests to verify detector creation

This ensures both the old (MonitorValidator) and new (detector API) flows
correctly create detectors for all monitors.

Part of NEW-593: Cron Monitor Alerts are not linked to monitor when created via the old flow

[sentry]

Bug: ensure_cron_detector() can return None, leading to an AssertionError at assert detector during monitor creation.
_{Severity: CRITICAL | Confidence: High}

🔍 Detailed Analysis

The ensure_cron_detector() function can return None if Detector.objects.get() fails (e.g., DoesNotExist) when trying to retrieve an existing detector. This can occur if a DataSource exists without a linked Detector, or due to a mismatch in detector creation logic. When ensure_cron_detector() returns None, the subsequent assert detector at src/sentry/monitors/validators.py:382 will raise an AssertionError, causing an unhandled HTTP 500 error in the monitor creation endpoint and potentially leaving orphaned monitors.

💡 Suggested Fix

Replace assert detector with an explicit if detector is None: check. Handle the None case gracefully by raising a specific validation error or returning an appropriate error response, rather than crashing the server with an AssertionError.

🤖 Prompt for AI Agent

Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: src/sentry/monitors/validators.py#L382

Potential issue: The `ensure_cron_detector()` function can return `None` if
`Detector.objects.get()` fails (e.g., `DoesNotExist`) when trying to retrieve an
existing detector. This can occur if a `DataSource` exists without a linked `Detector`,
or due to a mismatch in detector creation logic. When `ensure_cron_detector()` returns
`None`, the subsequent `assert detector` at `src/sentry/monitors/validators.py:382` will
raise an `AssertionError`, causing an unhandled HTTP 500 error in the monitor creation
endpoint and potentially leaving orphaned monitors.

_{Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 3856920}

[cursor]

Bug: Assert statement in production code causes crash

The assert detector statement will crash with an AssertionError when ensure_cron_detector returns None (which happens when database errors occur or a detector is missing). Assert statements should not be used for error handling in production code. This prevents graceful error handling and will cause monitor creation to fail unexpectedly.

 

[evanpurkhiser]

That's fine this should probably be a hard fail

[cathteng]

Do you need to return None? Isn't it always bad if you don't create or find the appropriate detector

[evanpurkhiser]

Right I could probably change this to be an exception. I’ll clean that up

[codecov]

Codecov Report

❌ Patch coverage is 70.00000% with 3 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
src/sentry/monitors/utils.py	60.00%	2 Missing ⚠️
src/sentry/monitors/validators.py	80.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           master   #104004      +/-   ##
===========================================
- Coverage   80.62%    80.60%   -0.02%     
===========================================
  Files        9317      9317              
  Lines      397599    397702     +103     
  Branches    25393     25393              
===========================================
+ Hits       320549    320568      +19     
- Misses      76598     76682      +84     
  Partials      452       452              

[linear]

NEW-593 Cron Monitor Alerts are not linked to monitor when created via the old flow