Repository navigation
feat(alerts): configurable quiet-hours / maintenance windows - #528
AbdulmalikAlayande merged 1 commit into
Conversation
…oLabs#325) Adds opt-in per-alert-config quiet-hours windows so operators can suppress alert delivery during planned maintenance without deleting and re-creating alert configs. ## Changes ### Schema & migration - src/db/schema.sql: add nullable quiet_hours_start, quiet_hours_end, quiet_hours_timezone TEXT columns to alert_configs (fresh installs) - src/db/migrations/002_quiet_hours.sql: ALTER TABLE migration for existing on-disk databases - src/db/migrator.ts: handle 'duplicate column name' errors gracefully for ADD COLUMN migrations (columns already present via schema.sql on fresh/test DBs) — marks migration applied rather than failing - src/db/database.ts: add quiet-hours columns to live migrations array (try/catch pattern); include new columns in table-rebuild functions migrateAlertConfigsChannelTypeCheck() and relaxChannelTypeChecks() ### Repository layer - src/db/repositories.ts: extend AlertConfig interface with quiet_hours_start/end/timezone; update insertAlertConfig() to accept and persist them; extend UndeliveredAlert interface and getUndeliveredAlerts() SQL to carry the fields to the dispatcher ### Dispatcher - src/alerts/dispatcher.ts: export isInQuietHours() and currentHHMMInTz() helpers using Intl.DateTimeFormat (no new dep); add quiet-hours skip check in deliverPendingAlerts() — skipped alerts remain pending (delivered=0, retry_count unchanged) for the next cycle ### CLI - src/commands/alerts.ts: add --quiet-hours <HH:MM-HH:MM> and --timezone <IANA-tz> flags to 'alerts add'; validates both are provided together, enforces HH:MM-HH:MM format, validates IANA tz via Intl; success message includes quiet window when set ### Tests (TDD) - tests/alerts/dispatcher.test.ts: extend seedAlert() helper to accept quiet-hours opts; add 'Quiet hours' describe block with 7 tests: no-quiet-hours delivery, inside-window skip (not delivered, not marked delivered, retry_count not incremented), outside-window delivery, overnight windows (start > end), pending-after-skip then delivery once window cleared, retry_count isolation, multi-alert isolation Closes TegoLabs#325
|
@temi-Dee Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds per-alert quiet-hours configuration with timezone validation, database persistence, migration handling, and delivery deferral. Alerts remain pending during active windows without retry increments, while alerts outside the window continue through normal delivery. ChangesQuiet-hours alert delivery
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Operator
participant AlertsCLI
participant Database
participant Dispatcher
participant Channel
Operator->>AlertsCLI: configure quiet-hours and timezone
AlertsCLI->>Database: store alert configuration
Dispatcher->>Database: load pending alert configuration
Dispatcher->>Dispatcher: evaluate current time in configured timezone
Dispatcher->>Channel: deliver when quiet window is inactive
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/commands/alerts.ts`:
- Around line 40-47: Update the resource-alert handling in the command flow
associated with the --quiet-hours and --timezone options so these values are not
silently ignored. Either validate and persist both options for resource alerts
consistently with the TTL branch, or explicitly reject the flags when resource
limits are selected; preserve existing behavior when the options are absent.
- Around line 119-126: Update the quiet-hours validation in the options parsing
flow to validate clock ranges as well as the HH:MM-HH:MM shape: restrict each
hour to 00–23 and each minute to 00–59 before persisting the setting. Keep the
existing invalid-input error message and process.exit behavior unchanged.
In `@src/db/database.ts`:
- Around line 113-115: Update both alert_configs rebuild copy queries: the
alert_configs_new query at src/db/database.ts lines 113-115 and the
alert_configs_relaxed query at lines 159-161 must include quiet_hours_start,
quiet_hours_end, and quiet_hours_timezone in matching INSERT and SELECT column
lists so configured quiet-hours values are preserved.
In `@src/db/migrator.ts`:
- Around line 88-111: Update the migration error handling around runMigrationTx
so an ADD COLUMN migration is marked applied only after verifying every target
column exists via PRAGMA table_info. Parse each ALTER TABLE statement, confirm
its table and column postconditions, and rethrow the original error if any
column is still missing; preserve the existing INSERT OR IGNORE recording only
for fully satisfied migrations.
In `@tests/alerts/dispatcher.test.ts`:
- Around line 455-498: Update the affected dispatcher tests to use Vitest fake
timers and set a fixed system time per test, replacing windows derived from the
real current time with deterministic, independently chosen expectations. Remove
the unused utcAtLocalTime helper unless it is needed to configure the fixed test
time, and ensure the outside, overnight, and always-quiet cases cannot depend on
the runtime minute or duplicate isInQuietHours logic.
- Around line 500-510: Remove the nested mockChannel definition in this describe
block and reuse the existing top-level mockChannel helper when initializing
channels in beforeEach, preserving the AlertChannel-typed channel map.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 1b0e2e95-2f77-4323-bdd6-08cf9c67fc05
📒 Files selected for processing (8)
src/alerts/dispatcher.tssrc/commands/alerts.tssrc/db/database.tssrc/db/migrations/002_quiet_hours.sqlsrc/db/migrator.tssrc/db/repositories.tssrc/db/schema.sqltests/alerts/dispatcher.test.ts
📜 Review details
🔇 Additional comments (6)
src/db/schema.sql (1)
51-56: LGTM!src/db/migrations/002_quiet_hours.sql (1)
1-16: LGTM!src/db/database.ts (1)
73-76: LGTM!src/db/repositories.ts (1)
49-54: LGTM!Also applies to: 282-294, 659-664, 697-700
src/alerts/dispatcher.ts (1)
24-76: LGTM!Also applies to: 106-127
tests/alerts/dispatcher.test.ts (1)
18-34: LGTM!Also applies to: 52-61
|
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
|---|---|---|---|---|---|
| - | - | Generic High Entropy Secret | ded54f4 | tests/commands/guard-cli-export-import.test.ts | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secret safely. Learn here the best practices.
- Revoke and rotate this secret.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
… rebuild (fixup for #528) migrateAlertConfigsChannelTypeCheck's and relaxChannelTypeChecks's table-rebuild INSERT/SELECT lists didn't include quiet_hours_start/end/timezone, so any existing database still carrying the legacy channel_type CHECK enum would silently drop configured quiet-hours windows the first time it opened after upgrading. Caught by CodeRabbit on #528; fixing directly since it touches the same migration functions from earlier this session.
…n (fixup for #518) 002_contract_groups.sql (this PR) and 002_quiet_hours.sql (merged separately as part of #528) both claimed migration version 2 - schema_migrations.version is an INTEGER PRIMARY KEY, so only one file can ever legitimately own that slot. It happens not to crash today only by accident: quiet_hours_start/end/ timezone are also declared directly in schema.sql (not just the migration file), so 002_quiet_hours.sql's ADD COLUMN statements fail with "duplicate column name" regardless of which file ran first, which the Migrator's existing idempotency catch already swallows. That's a coincidence, not a guarantee - the next migration to collide on a shared version number without that same redundant schema.sql declaration would crash getDatabase() for every fresh install. Renumbered to 003_contract_groups.sql.
… rebuild (fixup for #528) migrateAlertConfigsChannelTypeCheck's and relaxChannelTypeChecks's table-rebuild INSERT/SELECT lists didn't include quiet_hours_start/end/timezone, so any existing database still carrying the legacy channel_type CHECK enum would silently drop configured quiet-hours windows the first time it opened after upgrading. Caught by CodeRabbit on #528; fixing directly since it touches the same migration functions from earlier this session.
…n (fixup for #518) 002_contract_groups.sql (this PR) and 002_quiet_hours.sql (merged separately as part of #528) both claimed migration version 2 - schema_migrations.version is an INTEGER PRIMARY KEY, so only one file can ever legitimately own that slot. It happens not to crash today only by accident: quiet_hours_start/end/ timezone are also declared directly in schema.sql (not just the migration file), so 002_quiet_hours.sql's ADD COLUMN statements fail with "duplicate column name" regardless of which file ran first, which the Migrator's existing idempotency catch already swallows. That's a coincidence, not a guarantee - the next migration to collide on a shared version number without that same redundant schema.sql declaration would crash getDatabase() for every fresh install. Renumbered to 003_contract_groups.sql.
Summary
Implements issue #325. Adds opt-in per-alert-config quiet hours (maintenance windows) so operators can suppress alert delivery during planned downtime without deleting and re-creating alert configs.
During a quiet window, alerts remain pending (not marked delivered, retry_count not incremented) and will be picked up on the next daemon cycle once the window closes.
Changes
Schema & Migration
src/db/schema.sql— adds nullablequiet_hours_start,quiet_hours_end,quiet_hours_timezone(TEXT) toalert_configsfor fresh installssrc/db/migrations/002_quiet_hours.sql—ALTER TABLE ADD COLUMNmigration for existing on-disk databasessrc/db/migrator.ts—Migrator.run()now gracefully handles duplicate column name errors for ADD COLUMN migrations (columns already present from schema.sql on test/fresh DBs) and still marks them appliedsrc/db/database.ts— quiet-hours columns added to the live-migrations try/catch block;migrateAlertConfigsChannelTypeCheck()andrelaxChannelTypeChecks()updated to include new columns in table-rebuildsRepository Layer
src/db/repositories.ts—AlertConfiginterface andinsertAlertConfig()extended;UndeliveredAlertinterface andgetUndeliveredAlerts()SQL updated to surface quiet-hours fields to the dispatcherDispatcher
src/alerts/dispatcher.ts— exportsisInQuietHours()andcurrentHHMMInTz()helpers (built-inIntl.DateTimeFormat, no new dependency); skip check indeliverPendingAlerts()before the send call — overnight windows (22:00–06:00) handled correctlyCLI
src/commands/alerts.ts—alerts addaccepts--quiet-hours <HH:MM-HH:MM>and--timezone <IANA-tz>; validates both flags are provided together, enforces format, validates timezone viaIntlTests
7 new tests in
tests/alerts/dispatcher.test.ts(describe: Quiet hours):All 77 test files pass (1002 tests, 1 pre-existing skip, 0 failures).
Acceptance Criteria
Closes #325