feat(alerts): add scheduleStartAt + scheduleOffsetMinutes

[mlsalcedo]

Closes #1715

Summary

• Add scheduleStartAt and scheduleOffsetMinutes to alert schemas and API validation.
• Update alert evaluation scheduling to anchor windows by scheduleStartAt when set.
• Skip evaluations before scheduleStartAt.
• Keep current behavior unchanged when scheduling fields are unset.
• Add UI fields and API/OpenAPI/external API support for the new schedule options.
• Add alert scheduler tests for anchored windows and pre-start skip behavior.

Notes

• This enables Splunk-style scheduled monitor migration where checks must run on isolated, periodic windows anchored to specific times.
• scheduleStartAt is the primary anchor; scheduleOffsetMinutes remains optional for backward-compatible alignment.

[vercel]

@melsalcedo is attempting to deploy a commit to the HyperDX Team on Vercel.

A member of the Team first needs to authorize it.

[changeset-bot]

🦋 Changeset detected

Latest commit: d302304

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages

Name	Type
@hyperdx/api	Minor
@hyperdx/app	Minor
@hyperdx/common-utils	Minor
@hyperdx/otel-collector	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[sgarfinkel]

Left some feedback

[github-actions]

PR Review: feat(alerts): add scheduleStartAt + scheduleOffsetMinutes

No critical issues found. Implementation is well-structured with solid test coverage.

A few items worth noting:

• WARNING: validateAlertScheduleOffsetMinutes skips offset validation when scheduleStartAt is set — The check scheduleOffsetMinutes >= intervalMinutes still runs even when scheduleStartAt is provided (though it correctly does nothing at that point since offset is forced to 0). Not a bug, just a minor clarity issue.

• WARNING: Date range limits use approximate year lengths (365 * 24 * 60 * 60 * 1000) — won't account for leap years. Negligible in practice but worth noting.

• INFO: Redundant max(1439) on base Zod schema in zod.ts and types.ts — the superRefine already enforces the dynamic per-interval max. Not harmful, just redundant.

• INFO: makeAlert calls new Date(alert.scheduleStartAt) without error handling — safe because alertSchema.parse() is now called before makeAlert in all routers, ensuring scheduleStartAt is a valid ISO string. The recent change to explicitly parse in routers (previously req.body was used directly) is a good fix.

The backward-compat strategy via normalizeNoOpAlertScheduleFields and the null-vs-undefined distinction for scheduleStartAt serialization are well thought out.

Reviewed by Claude Sonnet 4.6

[github-actions]

Code Review

• ❌ Critical schema bug in packages/api/src/utils/zod.ts → The new .superRefine(validateAlertScheduleOffsetMinutes) is inserted with a semicolon before the existing .and(zSavedSearchAlert.or(zTileAlert)) line, making the trailing .and(...) a dead no-op expression. The alertSchema ends up missing the source discriminator after superRefine, meaning savedSearchId/dashboardId/tileId validation is silently skipped on the internal API. Remove the stale .and(zSavedSearchAlert.or(zTileAlert)) line that follows the semicolon.

• ⚠️ normalizeNoOpAlertScheduleFields silently drops scheduleOffsetMinutes: 0 reset (packages/app/src/utils/alerts.ts) → When previousAlert is null/undefined and the user explicitly sets offset to 0, previousHadOffset is false so the field is deleted instead of being sent to the API to clear a stale DB value. The user's explicit reset is silently ignored.

• ⚠️ (alert as any) casts for createdBy in DBEditTimeChartForm.tsx → Extend the local alert type to include createdBy instead of bypassing TypeScript.

• ⚠️ Offset-cap enforcement (scheduleOffsetMinutes < intervalMinutes) is only applied via superRefine → Since this sits after the broken schema chain (see item 1), invalid offsets can pass validation on the internal API. Fixing item 1 resolves this automatically.

• ℹ️ Missing test: scheduleOffsetMinutes surviving an update when scheduleStartAt is absent → Add a test to confirm the existing DB offset is not zeroed on a partial update that omits the offset field.

🤖 Generated with Claude Code

[teeohhem]

Thanks for this. I plan on finishing up my review Monday morning with some feedback.

[teeohhem]

things I added

1. Advanced Settings collapsable menu for these new fields, along with text descriptions and tooltips.
2. Date format in the timepicker to match the rest of the application.

[github-actions]

Claude finished @mlsalcedo's task in 2m 59s —— View job

---

Code Review

• Gather context and read changed files
• Analyze for critical issues
• Post review

---

Review

Previous critical issues from the last review have been resolved:

• ✅ Schema chain fixed — alertSchema in zod.ts now correctly chains .and(zSavedSearchAlert.or(zTileAlert)).superRefine(validateAlertScheduleOffsetMinutes). The savedSearchId/dashboardId/tileId discriminator validation is no longer a dead no-op.
• ✅ (alert as any) casts removed from DBEditTimeChartForm.tsx
• ✅ Silent offset-0 drop fixed — normalizeNoOpAlertScheduleFields now uses dirtyFields.alert?.scheduleOffsetMinutes as preserveExplicitScheduleOffsetMinutes consistently in both DBSearchPageAlertModal and DBEditTimeChartForm

Remaining items:

• ⚠️ Double-parse overhead (packages/api/src/routers/api/alerts.ts:103, packages/api/src/routers/external-api/v2/alerts.ts:419, 511) — alertSchema.parse(req.body) is called again inside the handler after validateRequest middleware already ran it. The comment explains the rationale (enforce refinements, strip unknown fields). This is safe but adds redundant work on every request; consider a shared parsed-body approach if this becomes a perf concern.

• ℹ️ name / message omitted from internal GET /api/alerts list (packages/api/src/routers/api/alerts.ts:67–79) — the _.pick doesn't include name or message. Pre-existing omission, not introduced by this PR.

• ℹ️ AlertScheduleFields accordion opened state (packages/app/src/components/AlertScheduleFields.tsx:58) — useState(hasAdvancedScheduleValues) captures the initial value only. If a parent re-renders with a different scheduleOffsetMinutes/scheduleStartAt, the accordion won't auto-open. Acceptable for current UX since initial form values are set correctly on mount.

✅ No blocking issues. Implementation is solid — backward-compat strategy, validation chain, and scheduler window logic are all correct.

[mlsalcedo]

• Advanced Settings collapsable menu for these new fields, along with text descriptions and tooltips.
• Date format in the timepicker to match the rest of the application.

Done

[github-actions]

Claude finished @mlsalcedo's task in 1m 37s —— View job

---

Code Review

• Gather context and read changed files
• Analyze for critical issues
• Post review

---

Review

Previous critical issues have been resolved. The implementation looks solid — schema chain, type safety, and backward-compat strategy are all correct.

A few remaining items:

• ⚠️ scheduleStartAt: null stored explicitly in MongoDB (packages/api/src/models/alert.ts) — When a user clears scheduleStartAt, null is persisted to the document (vs. the key being absent on pre-migration alerts). checkAlerts handles this correctly via scheduleStartAt == null, but any future queries filtering { scheduleStartAt: { $exists: false } } would miss these documents. Low risk now, worth noting for migrations.

• ⚠️ getScheduledWindowStart exported but has no test for scheduleOffsetMinutes > 0 with anchored start (packages/api/src/tasks/checkAlerts/index.ts:113) — The function logs that offset is ignored when scheduleStartAt is set, but there's no unit test confirming scheduleOffsetMinutes is actually ignored in the return value. The processAlert path normalizes this, but getScheduledWindowStart itself doesn't enforce it.

• ℹ️ processRequest vs validateRequest switch (packages/api/src/routers/api/alerts.ts, external-api/v2/alerts.ts) — Good change. processRequest replaces req.body with the parsed+stripped result, enforcing refinements (including superRefine) where validateRequest did not. The behavior change is intentional and correct.

• ℹ️ useState(hasAdvancedScheduleValues) initial capture (packages/app/src/components/AlertScheduleFields.tsx:58) — Pre-existing from previous review: accordion won't auto-open if parent re-renders with new schedule values post-mount. Acceptable for current UX.

✅ No blocking issues. Ready to merge pending reviewer approval.

[github-actions]

Claude finished @mlsalcedo's task in 1m 37s —— View job

---

Code Review

• Gather context and read changed files
• Analyze for critical issues
• Post review

---

Review

All critical issues from previous reviews have been resolved. The implementation is solid — schema chain, type safety, backward-compat strategy, and scheduler window logic are correct.

Remaining items (carry-overs from prior reviews):

• ⚠️ getScheduledWindowStart accepts scheduleOffsetMinutes even when scheduleStartAt is set (packages/api/src/tasks/checkAlerts/index.ts:249–271) — the function itself doesn't drop the offset; only processAlert enforces the ignore-behavior. If called directly with both params, it silently applies offset anyway. No test confirms offset is ignored at the return-value level. Low risk since callers normalize, but a comment or assertion inside the function would help.

• ℹ️ scheduleStartAt: null written explicitly to MongoDB on clear (packages/api/src/models/alert.ts) — future queries using { $exists: false } would miss these docs. Pre-migration alerts use key-absent, post-clear alerts use null. Consider coercing to key-absent on read/write if this becomes a migration concern.

• ℹ️ useState(hasAdvancedScheduleValues) captures initial value only (packages/app/src/components/AlertScheduleFields.tsx:58) — accordion won't auto-open if parent passes new schedule values post-mount. Acceptable for the current UX since initial form values are set correctly.

✅ No blocking issues. Ready to merge pending reviewer approval.

[teeohhem]

@claude please review and fix the conflicts

[github-actions]

Claude Code is working…

I'll analyze this and get back to you.

View job run

[github-actions]

Claude finished @mlsalcedo's task in 1m 36s —— View job

---

Code Review

• Check for merge conflicts
• Review code for critical issues

---

Conflict Status

The conflicts previously flagged by GitHub were already resolved in merge commit d1c329a5 ("Merge remote-tracking branch 'upstream/main' into codex/issue-1715-starttime-offset"). The branch is fully up-to-date with origin/main — no pending conflicts remain.

The conflict resolution in checkAlerts/index.ts and checkAlerts/__tests__/checkAlerts.test.ts correctly integrates both the new scheduling logic (getScheduledWindowStart, alertHasGroupBy, scheduleStartAt pre-skip) and the upstream whereLanguage fixes for tile alerts.

Review

✅ No critical issues. The implementation is solid and previous review blockers have all been addressed.

One carry-over worth noting before merge:

• ⚠️ getScheduledWindowStart doesn't enforce offset=0 when scheduleStartAt is set (packages/api/src/tasks/checkAlerts/index.ts:150–168) — the function accepts both params but only processAlert enforces the ignore-behavior via a log warning. If called directly with both, offset is silently applied. A guard or comment inside the function would prevent future misuse. Fix this →

[github-actions]

Claude finished @mlsalcedo's task in 37s —— View job

---

Code Review

• Read changed files
• Analyze for critical issues
• Post review

---

Review

All previously flagged critical issues have been resolved. The implementation looks correct.

One minor carry-over worth noting:

• ⚠️ getScheduledWindowStart silently accepts scheduleOffsetMinutes when scheduleStartAt is set (packages/api/src/tasks/checkAlerts/index.ts:148–169) — if called directly with both params, offset is applied anyway; only processAlert enforces ignore-behavior. A guard or comment inside the function would prevent future misuse. Fix this →

✅ No blocking issues. Ready to merge pending reviewer approval.

[github-actions]

Claude encountered an error —— View job

---

I'll analyze this and get back to you.