Skip to content

feat(alerts): open and resolve pagerduty incidents for insight alerts - #127898

Merged
trunk-io[bot] merged 2 commits into
masterfrom
claude/gallant-fermat-1b3a4e
Nov 3, 2026
Merged

trunk-io[bot] merged 2 commits into
masterfrom
claude/gallant-fermat-1b3a4e

Conversation

@jzhu13

@jzhu13 jzhu13 commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

Problem

  • An insight alert that pages PagerDuty never resolves its incident, so on-call closes every page by hand.
  • Logs, metrics and warehouse sync alerts on the alerts platform already open and resolve incidents. Insight alerts send no incident actions.
  • Insight alerts announce no resolve message by design. That stays.

Changes

  • A platform-owned insight alert now opens a PagerDuty incident when it starts firing and resolves it when it stops.
  • The destination listens on two new events, $insight_alert_incident_opened and $insight_alert_incident_closed.
  • Slack, webhook, email and in-app destinations still get no resolve message.
  • The incident follows the alert's state, not its announcement, so a resolve goes out although nothing is announced.
  • A disable, delete, snooze or edit through either API also closes the incident.
  • A failed or broken check keeps the incident open, because the firing stays open.
  • A PagerDuty function on $insight_alert_firing keeps its HogFunction path. It still triggers and never resolves.
  • Alerts the insight stack still owns are unchanged.
  • The insight API's destination delete endpoint also removes functions on the incident events. The generic HogFunction API refuses to edit them.
  • No UI yet. The insight alert form gets a PagerDuty option in a follow-up.
flowchart LR
  A[Insight check stops firing]:::phYellow --> B[No announcement]:::phGray --> C[No delivery]:::phGray
  classDef phYellow fill:#f9bd2b,stroke:#f9bd2b,color:#000;
  classDef phGray fill:#e5e7eb,stroke:#c7ccd1,color:#000;
Loading
flowchart LR
  A[Insight check stops firing]:::phYellow --> B[Incident action: resolve]:::phBlue --> C[Delivery with no messages]:::phBlue --> D[PagerDuty resolve]:::phRed
  classDef phYellow fill:#f9bd2b,stroke:#f9bd2b,color:#000;
  classDef phBlue fill:#1d4aff,stroke:#1d4aff,color:#fff;
  classDef phRed fill:#f54e00,stroke:#f54e00,color:#fff;
Loading

How did you test this code?

Test rationale:

  • test_only_an_allowlisted_or_platform_owned_firing_failure_or_incident_edge_asks_for_a_delivery gains paged cases. It catches _delivery dropping a resolve because nothing was announced. Removing that path fails paged_resolve.
  • test_an_incident_edge_emits_its_incident_event_beside_any_message catches the HogFunction-path emitter dropping incident events.
  • test_an_insight_pagerduty_function_is_native_only_on_the_incident_events (database-backed) catches event ids not read from the function's filters. That would send firing-event functions natively, where every PagerDuty send fails.
  • test_an_incident_destination_is_removed_through_the_insight_api catches the delete endpoint ignoring incident-event functions, which nothing else can remove. It fails without the fix.
  • Not checked: a live PagerDuty send, and dogfooding on project 2.

Release status

  • No feature flag controls this change

Docs update

  • Added a line on insight incidents to docs/internal/alerts-platform-temporal.md, under "Incident edges ignore cooldown and mute".

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

Agent: Claude Code, Claude Opus 5.5 (claude-opus-5-5)

Skills: /writing-tests, /querying-production-databases-via-metabase, /reviewing-with-coderabbit, /simplify, /code-review, /writing-pr-descriptions

More context for AI agents
  • Decisions: native incidents for platform-owned alerts only. The insight stack's own sends are untouched, and shadow checks never page.
  • Decisions: existing PagerDuty functions on the firing event are not migrated. They keep the HogFunction path.
  • Decisions: a broken check no longer closes the incident, because the instance stays firing and a re-enabled alert would never re-trigger.
  • Reviews: CodeRabbit --deep reported 0 findings. /simplify and /code-review high --fix fixes are applied. PostHog Review's one finding (delete endpoint) is fixed.
  • Follow-ups: PagerDuty option in the insight alert form, offered only on platform-owned alerts.
  • Follow-ups: a source-agnostic PagerDuty hold-back, and one shared "has an incident destination" helper across sources.
  • Read closely: _incident_actions in platform_source_cycle.py, and the narrowed rule in logic/delivery_path.py.

🤖 Generated with Claude Code

Insight alerts the platform owns now send incident actions, so a
PagerDuty destination on the new insight incident events triggers when
the alert starts firing and resolves when it stops. Message destinations
still hear no resolve. A PagerDuty function on the firing event keeps
its HogFunction path.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@jzhu13 jzhu13 self-assigned this Nov 2, 2026
@trunk-io

trunk-io Bot commented Nov 2, 2026 •

Copy link
Copy Markdown

😎 Merged successfully - details.

@github-actions

github-actions Bot commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

🤖 CI report

⚠️ Trunk lane — backend Python lane

This PR is assigned to the backend Python lane. It runs backend Python tests and may merge in parallel with PRs in other lanes.

✅ Duplication (Python) — clean

New Python code duplication introduced by this branch. Fails at 70+ tokens in app code, or 150+ tokens when both copies live in test files. Advisory while the gate proves itself: extract a shared helper instead of copying.

✅ Duplication (TypeScript) — clean

New TypeScript code duplication introduced by this branch. Fails at 70+ tokens in app code, or 150+ tokens when both copies live in test files. Advisory while the gate proves itself: extract a shared helper instead of copying.

@posthog posthog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PostHog Review · Standard · reviewed 1c7e6da: 1 Should fix.

This is the only automatic review while this pull request is a draft. Comment @posthog review to review new changes sooner. Otherwise, a review of the new changes runs when you mark it ready for review.

See this review and its findings in PostHog Review. Comment @posthog review --deep for a Deep review. A Deep review takes longer and looks deeper than a Standard review.

@jzhu13
jzhu13 marked this pull request as ready for review November 3, 2026 00:33
@parameterai

parameterai Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

Risk: No findings

This delta widens the insight alert API's destination-delete allowlist to include the two new incident events, so a PagerDuty trigger/resolve pair can be removed through the insight API (the generic HogFunction API refuses edits to managed alert events). The deletion stays gated by team scoping, alert:write permission, an alert_id pin in the function's filters, a known-template filter, and pair-wise group deletion, so I found no new security risk.

Sentinel reviewed e3a63b9 · Review settings

@pr-assigner-resolver-posthog
pr-assigner-resolver-posthog Bot requested a review from a team November 3, 2026 00:33
The generic HogFunction API treats the insight incident events as
managed and refuses to edit them, so the insight destinations delete
endpoint now matches functions on those events too.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@jzhu13 jzhu13 added the stamphog Request AI approval (no full review) label Nov 3, 2026

@stamphog stamphog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved.

The risky delivery and API changes have owning-team and strong-familiarity assurance, plus a current-head review reported without findings. The destination deletion fix is present, and no unresolved substantive concerns or other refusal grounds were found.

  • Author wrote 98% of the modified lines and has 28 merged PRs in these paths (familiarity STRONG).
  • PostHog Review (Standard) reviewed the current head: no findings.
Gate mechanics and policy version
Gate Result
prerequisites ✓ all clear
deny-list ✓ no deny categories matched
size ✓ 138L, 7F substantive, 347L/13F incl. docs/generated/snapshots — within ceiling
tier ✓ T1-agent / T1d-complex (347L, 13F, two-areas, feat)
stamphog 2.7.0 .stamphog/policy.yml @ e3a63b9 · reviewed head e3a63b9

@jzhu13

jzhu13 commented Nov 3, 2026

Copy link
Copy Markdown
Contributor Author

/trunk merge

@jzhu13
jzhu13 added this pull request to stack #127918 November 3, 2026 01:10
@trunk-io
trunk-io Bot merged commit e9675d5 into master Nov 3, 2026
330 of 331 checks passed
@trunk-io
trunk-io Bot deleted the claude/gallant-fermat-1b3a4e branch November 3, 2026 01:20
@deployment-status-posthog

deployment-status-posthog Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

Deploy status

Environment Status Deployed At Workflow
dev ✅ Deployed 2026-11-03 01:45 UTC Run
prod-us ✅ Deployed 2026-11-03 02:06 UTC Run
prod-eu ✅ Deployed 2026-11-03 02:02 UTC Run

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

stamphog Request AI approval (no full review)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant