ref(aci): decouple detector from workflowfirehistory in API

[cathteng]

Since we now have DetectorGroup rows being written, we could refactor the WorkflowGroupHistory API to use DetectorGroup, rather than use a column on WorkflowFireHistory (which shouldn't need to know which detector triggered the workflow to be fired).

Don't use DetectorWorkflow to check for active detector - workflow connections because a detector might have been disconnected from the workflow since the workflow was triggered for that issue.

[cathteng]

Having second thoughts on whether we should do this because the detector in the notification is so tightly coupled with the metric detector code (charts, open periods, etc)

[cathteng]

OK we back. We will decouple detector from action.trigger here #103000

[sentry]

Bug: The DetectorGroup subquery fails for old groups lacking DetectorGroup entries, causing incorrect detector_id reporting.
_{Severity: CRITICAL | Confidence: 0.98}

🔍 Detailed Analysis

The detector_subquery in workflow_group_history_serializer.py assumes that a DetectorGroup entry exists for every WorkflowFireHistory record with a detector_id. This assumption is false for groups created before the introduction of DetectorGroup entries. When a workflow fires for such an 'old' group, a WorkflowFireHistory record is created with a detector_id, but no corresponding DetectorGroup entry exists. Consequently, the subquery returns None, leading the API to incorrectly report detector=None for these groups, despite a detector having fired the workflow.

💡 Suggested Fix

Modify the detector_subquery in workflow_group_history_serializer.py to correctly retrieve the detector_id for groups that lack a DetectorGroup entry, potentially by falling back to WorkflowFireHistory.detector_id or ensuring DetectorGroup entries are backfilled.

🤖 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/workflow_engine/endpoints/serializers/workflow_group_history_serializer.py#L130-L141

Potential issue: The `detector_subquery` in `workflow_group_history_serializer.py`
assumes that a `DetectorGroup` entry exists for every `WorkflowFireHistory` record with
a `detector_id`. This assumption is false for groups created before the introduction of
`DetectorGroup` entries. When a workflow fires for such an 'old' group, a
`WorkflowFireHistory` record is created with a `detector_id`, but no corresponding
`DetectorGroup` entry exists. Consequently, the subquery returns `None`, leading the API
to incorrectly report `detector=None` for these groups, despite a detector having fired
the workflow.

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

[semgrep-code-getsentry]

High severity vulnerability may affect your project—review required:
Line 125 lists a dependency (django) with a known High severity vulnerability.

ℹ️ Why this matters

Affected versions of Django are vulnerable to Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection'). SQL injection in Django's ORM column aliases: when using QuerySet.annotate(), QuerySet.alias(), QuerySet.aggregate(), or QuerySet.extra() with dictionary expansion (**kwargs), the dictionary keys are used unescaped as SQL column aliases. On MySQL and MariaDB backends, an attacker who can influence those keys (for example, by passing a crafted dict of annotations) can inject arbitrary SQL into the generated query.

References: GHSA, CVE

To resolve this comment:
Check if you are using Django with MySQL or MariaDB.

• If you're affected, upgrade this dependency to at least version 5.2.7 at uv.lock.
• If you're not affected, comment /fp we don't use this [condition]

💬 Ignore this finding

To ignore this, reply with:

• /fp <comment> for false positive
• /ar <comment> for acceptable risk
• /other <comment> for all other reasons

_{You can view more details on this finding in the Semgrep AppSec Platform here.}