fix: add private locations incident trigger

[zapteryx]

Resolves #2502

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

3 Skipped Deployments

Project	Deployment	Actions	Updated (UTC)
openstatus-dashboard	Skipped		Jul 30, 2026 8:09am
openstatus-status-page	Skipped		Jul 30, 2026 8:09am
openstatus-web	Skipped		Jul 30, 2026 8:09am

[cubic-dev-ai]

Review completed against the latest diff

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[cubic-dev-ai]

All reported issues were addressed across 1 file (changes from recent commits).

<sub>Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic</sub>

[cubic-dev-ai]

All reported issues were addressed across 1 file (changes from recent commits).

<sub>Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic</sub>

[cubic-dev-ai]

All reported issues were addressed across 1 file (changes from recent commits).

<sub>Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic</sub>

[thibaultleouay]

@claude review this

[claude]

Claude finished @thibaultleouay's task in 2m 10s —— View job

---

Review: private location incident triggering

• Gather context (diff vs origin/main, prior review rounds)
• Read changed files in full (incident-utils.ts, private-location.ts, index.ts)
• Review for bugs, race conditions, style/consistency
• Post review feedback

Re-reviewed the current state (a833b41f) against origin/main. Good news: the two P1 blockers from my last pass on this PR are now fixed.

Resolved since last review

• The degraded and active branches in private-location.ts (lines 326–344, 370–385) both now call the shared resolveIncident() unconditionally when shouldTriggerIncident is true, instead of gating on monitor.status === "error" (a field this file never writes) or duplicating logic with a manual findOpenIncident + update. Both paths are consistent now.
• resolveIncident() in incident-utils.ts does a single atomic bulk UPDATE ... WHERE id IN (...) AND resolved_at IS NULL RETURNING *, so it can't leave a second open incident behind and can't double-resolve under concurrent reports.
• index.ts now imports the same findOpenIncident/resolveIncident from incident-utils.ts instead of keeping a second near-identical copy — addresses cubic's duplication comment.
• Error logging in the new catch blocks is now consistent with the rest of the file (error_message: error instanceof Error ? error.message : String(error)).

Still open — test coverage gap (P2)

apps/workflows/src/checker/private-location.test.ts:97 — TEST_MONITOR_ID is created with regions: "ams", a non-empty cloud region. In private-location.ts, hasCloudRegions is derived from that field and shouldTriggerIncident = !hasCloudRegions && (...). With this fixture, shouldTriggerIncident is always false, so none of the incident create/resolve code this PR adds (private-location.ts:237-299 error, :325-343 degraded, :368-385 active) is exercised by any test in this file — including "recovery after error" (:282) and "degraded report" (:304), which only assert on notification calls, not on incidentTable rows. This is exactly the gap that let the earlier monitor.status === "error" dead-code bug ship unnoticed for several review rounds.

Suggest adding a private-only monitor fixture (empty regions) and asserting incidentTable rows across: error → creates incident, degraded-with-majority → resolves it, active → resolves it.
Fix this →

Minor, non-blocking (P3)

packages/db/src/schema/incidents/incident.ts:65 — unique(monitorId, startedAt) only collides if two concurrent inserts share the exact same cronTimestamp; different-second reports can still create two open incidents for one monitor. This is mitigated (not eliminated) by resolveIncident() now resolving all open rows rather than assuming one, so it's not a correctness bug in practice, just a residual gap noted in earlier reviews. Worth a follow-up (partial unique index on monitorId WHERE resolved_at IS NULL) if duplicate incident.created audit rows/notifications on the same outage matter.

---

Summary: The core incident create/resolve logic is now correct and consistent between the degraded and active paths, and the atomic bulk resolve in incident-utils.ts handles the race conditions cubic flagged earlier. The one thing I'd still want before merge is test coverage for the actual feature (private-only monitor fixture), since right now the whole incident path is untested and that's precisely how the earlier P1 regression slipped through several review rounds.