feat(alerts): add per-channel enable/disable toggle without deleting the config

[YngPrince11]

Closes #326

[drips-wave]

@YngPrince11 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! 🚀

Learn more about application limits

[coderabbitai]

📝 Walkthrough

Summary by CodeRabbit

• New Features
  • Added alerts enable and alerts disable commands to control alert configurations by ID.
  • Added validation and clear errors for invalid or missing alert IDs.
• Bug Fixes
  • Disabled alert configurations no longer trigger new alerts or resolve existing ones.
• Migration
  • Existing alert configurations remain enabled by default, while their status can now be managed individually.

Walkthrough

Alert configurations now persist an enabled state, expose alerts enable and alerts disable commands, and skip disabled configurations during monitoring. Tests cover toggling, invalid IDs, and missing configurations.

Changes

Alert configuration toggles

Layer / File(s)	Summary
Persist enabled state src/db/migrations/..., src/db/schema.sql, src/db/repositories.ts	Adds the enabled column, documents its SQLite boolean representation, and provides a repository helper for updating it.
CLI toggle commands and validation src/commands/alerts.ts, tests/commands/alerts.test.ts	Adds enable/disable commands with numeric ID validation, existence checks, state updates, confirmations, and success/error tests.
Monitor enabled-state filtering src/core/monitor.ts	Skips disabled alert configurations during threshold-crossing and resolution processing.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Suggested reviewers: abdulmalikalayande

Poem

A rabbit flipped a toggle bright,
“Sleep, alert, through quiet night!”
The config stays, its bounds remain,
Then wakes to watch the stream again.
Hop, hop—alerts now obey!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Out of Scope Changes check	⚠️ Warning	src/db/repositories.ts adds a new helper outside the files explicitly listed in the issue scope.	Move the enabled-toggle DB update into a scope-approved path or confirm repositories.ts is in scope before merging.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly states the per-channel enable/disable toggle and matches the main change.
Description check	✅ Passed	The description is related to the PR and references the linked issue being closed.
Linked Issues check	✅ Passed	The PR implements the enabled column, monitor filtering, enable/disable commands, and tests required by #326.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 4

🤖 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 211-215: Replace parseInt-based validation in the enable-ID branch
of src/commands/alerts.ts lines 211-215 with strict Number(options.id)
validation requiring a positive safe integer, and apply the same validation in
the disable-ID branch at lines 234-238 before any configuration lookup or
mutation. Add tests covering malformed IDs such as prefixed text and decimals
for both commands.

In `@src/db/migrations/002_add_enabled_to_alert_configs.sql`:
- Line 1: Make the migration statement in
src/db/migrations/002_add_enabled_to_alert_configs.sql safe when
alert_configs.enabled already exists, while preserving its ability to add the
column for existing databases. Update the corresponding schema definition in
src/db/schema.sql only as needed to keep fresh-database initialization
consistent; both sites must remain compatible with Migrator.run().

In `@tests/commands/alerts.test.ts`:
- Around line 653-724: Remove the duplicated “alerts enable / disable” describe
suite containing the tests for disabling, re-enabling, missing IDs, and
non-numeric IDs; retain the other identical suite around the existing alerts
command tests so each behavior remains covered exactly once.
- Around line 542-607: Add an E2E monitor-cycle test in
tests/core/monitor.test.ts or tests/alerts/lifecycle.test.ts covering
setAlertConfigEnabled and the enabled: 0 state. Verify a disabled config neither
increments thresholdsCrossed nor resolves alerts, then re-enable it and confirm
the next eligible monitor cycle fires the alert.

🪄 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: d81ac804-682c-4e13-b335-bb50689fb4c6

📥 Commits

Reviewing files that changed from the base of the PR and between 35d9237 and afe80ba.

📒 Files selected for processing (6)

• src/commands/alerts.ts
• src/core/monitor.ts
• src/db/migrations/002_add_enabled_to_alert_configs.sql
• src/db/repositories.ts
• src/db/schema.sql
• tests/commands/alerts.test.ts

📜 Review details

🧰 Additional context used

🪛 Squawk (2.61.0)

src/db/migrations/002_add_enabled_to_alert_configs.sql

[warning] 1-1: Using 32-bit integer fields can result in hitting the max int limit. Use 64-bit integer values instead to prevent hitting this limit.

(prefer-bigint-over-int)

🔇 Additional comments (2)

src/db/repositories.ts (1)

44-48: LGTM!

Also applies to: 298-300

src/core/monitor.ts (1)

211-214: LGTM!

[coderabbitai]

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Reject malformed IDs instead of truncating them.

parseInt accepts prefixes, so --id 1junk or --id 1.5 targets config 1. Require a positive safe integer before looking up or mutating configuration state.

• src/commands/alerts.ts#L211-L215: validate enable IDs with Number.isSafeInteger(Number(options.id)) and > 0.
• src/commands/alerts.ts#L234-L238: apply the same strict validation to disable IDs and add malformed-ID tests.

📍 Affects 1 file

• src/commands/alerts.ts#L211-L215 (this comment)
• src/commands/alerts.ts#L234-L238

🤖 Prompt for 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.

In `@src/commands/alerts.ts` around lines 211 - 215, Replace parseInt-based
validation in the enable-ID branch of src/commands/alerts.ts lines 211-215 with
strict Number(options.id) validation requiring a positive safe integer, and
apply the same validation in the disable-ID branch at lines 234-238 before any
configuration lookup or mutation. Add tests covering malformed IDs such as
prefixed text and decimals for both commands.

[coderabbitai]

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
sed -n '34,82p' src/db/database.ts
rg -n -C3 'alert_configs|enabled|migrator\.run|new Migrator' src/db

Repository: AbdulmalikAlayande/sorokeep

Length of output: 24486

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '--- database.ts top ---\n'
sed -n '1,42p' src/db/database.ts

printf '\n--- package deps mentioning sqlite3/migrations ---\n'
if [ -f package.json ]; then
  cat package.json | jq '.dependencies, .devDependencies' 2>/dev/null || cat package.json
fi

printf '\n--- locate migrator implementation/files ---\n'
git ls-files | grep -Ei 'migrat|sqlite|database|schema' | sed -n '1,120p'

printf '\n--- imports of database.ts locations ---\n'
rg -n "from ['\"].*db/(database|repositories)|getDatabase\\(" src --glob '!node_modules/**'

Repository: AbdulmalikAlayande/sorokeep

Length of output: 256

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- database.ts top ---'
sed -n '1,42p' src/db/database.ts

printf '%s\n' '--- package deps mentioning sqlite3/migrations ---'
if [ -f package.json ]; then
  node -e 'const p=require("./package.json"); console.log(JSON.stringify({dependencies:p.dependencies||{},devDependencies:p.devDependencies||{}},null,2))' || cat package.json
fi

printf '%s\n' '--- locate migrator implementation/files ---'
git ls-files | grep -Ei 'migrat|sqlite|database|schema' | sed -n '1,120p'

printf '%s\n' '--- imports of database.ts locations ---'
rg -n "from ['\"].*db/(database|repositories)|getDatabase\\(" src --glob '!node_modules/**'

Repository: AbdulmalikAlayande/sorokeep

Length of output: 9505

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- src/db/migrator.ts ---'
cat -n src/db/migrator.ts

printf '%s\n' '--- tests/db/migrator.test.ts ---'
cat -n tests/db/migrator.test.ts | sed -n '1,220p'

printf '%s\n' '--- test setup / schema-related setup ---'
cat -n src/db/database.ts | sed -n '34,75p'

Repository: AbdulmalikAlayande/sorokeep

Length of output: 11865

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- fresh db behavior via SQL semantics probe ---'
node - <<'JS'
const Database = require('better-sqlite3');
const tmp = require('node:os').tmpdir();
const fs = require('node:fs');
const path = require('node:path');

const dbPath = path.join(tmp, 'sorokeep_probe_' + process.pid + '.db');
try {
  const db = new Database(dbPath);
  db.exec(`CREATE TABLE alert_configs (id INTEGER PRIMARY KEY); INSERT INTO alert_configs VALUES (4);`);
  try {
    db.exec(`ALTER TABLE alert_configs ADD COLUMN enabled INTEGER NOT NULL DEFAULT 1;`);
    db.exec(`ALTER TABLE alert_configs ADD COLUMN enabled INTEGER NOT NULL DEFAULT 1;`);
    console.log('FAIL: second add did not throw');
  } catch (err) {
    console.log('second add throws:', err.message);
  }
  try {
    db.exec(`ALTER TABLE alert_configs ADD COLUMN enabled INTEGER NOT NULL DEFAULT 0, created_at TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP;`);
    console.log('compound add throws:', 'unexpected');
  } catch (err) {
    console.log('compound add throws:', err.message);
  }
} finally {
  try { fs.unlinkSync(dbPath); } catch {}
}
JS

printf '%s\n' '--- check whether tests cover fresh db migrations/schema combination ---'
rg -n "alert_configs|enabled|alertConfig|fresh database|schema.sql|migrations" tests sources src package.json docs --glob '!node_modules/**' | sed -n '1,240p'

Repository: AbdulmalikAlayande/sorokeep

Length of output: 1078

---

🌐 Web query:

SQLite ALTER TABLE ADD COLUMN if column already exists error duplicate column

💡 Result:

SQLite does not support an IF NOT EXISTS clause for the ALTER TABLE ADD COLUMN statement [1][2]. If you attempt to add a column that already exists, SQLite will throw an error with the message duplicate column name: [column_name] [2][3]. Because there is no native syntax to handle this automatically, you must manage this condition in your application logic or migration scripts [1][4]. The following strategies are commonly used: 1. Check for the column using PRAGMA table_info: Before executing the ALTER TABLE command, query the table's metadata to see if the column exists. You can use the PRAGMA table_info(table_name) statement [1][2]. Example SQL logic: SELECT count(*) FROM pragma_table_info('your_table_name') WHERE name = 'your_column_name'; If the result is 0, the column does not exist and it is safe to proceed with your ALTER TABLE statement [1][2]. 2. Wrap in a try-catch block: If your application environment allows it, execute the ALTER TABLE statement and catch the specific exception or error code returned when the column already exists [1][2]. This allows your application to silently ignore the error if it indicates that the column is already present [5]. 3. Verify schema before migration: If you are managing database schema upgrades (e.g., in mobile development), ensure your versioning logic is correct so that migration scripts are not applied out of order or multiple times to the same database file [3][6]. The PRAGMA table_info approach is generally considered the most reliable method for creating idempotent migration scripts [2]. Attempting to parse the error message text from the database is discouraged as it is brittle and may change [5].

Citations:

• 1: https://stackoverflow.com/questions/3604310/alter-table-add-column-if-not-exists-in-sqlite
• 2: https://www.w3tutorials.net/blog/alter-table-add-column-if-not-exists-in-sqlite/
• 3: https://stackoverflow.com/questions/43356823/duplicate-column-name-on-sqlite-upgrade-for-a-new-column
• 4: https://stackoverflow.com/questions/17755931/sqlite-add-column-if-not-exist-with-just-sql
• 5: https://sqlite.org/forum/info/3fda139170250ce5
• 6: https://stackoverflow.com/questions/71950634/cause-of-fatal-exception-android-database-sqlite-sqliteexception-duplicate-colu

---

Don’t apply 1 002_add_enabled_to_alert_configs.sql on fresh databases.

getDatabase executes schema.sql before Migrator.run(), so a newly created database already has alert_configs.enabled and SQLite will fail on this ADD COLUMN. Keep the migration for existing databases that need it, but make its schema initialization effect safe before applying it again.

🧰 Tools

🪛 Squawk (2.61.0)

[warning] 1-1: Using 32-bit integer fields can result in hitting the max int limit. Use 64-bit integer values instead to prevent hitting this limit.

(prefer-bigint-over-int)

📍 Affects 2 files

• src/db/migrations/002_add_enabled_to_alert_configs.sql#L1-L1 (this comment)
• src/db/schema.sql#L51-L51

🤖 Prompt for 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.

In `@src/db/migrations/002_add_enabled_to_alert_configs.sql` at line 1, Make the
migration statement in src/db/migrations/002_add_enabled_to_alert_configs.sql
safe when alert_configs.enabled already exists, while preserving its ability to
add the column for existing databases. Update the corresponding schema
definition in src/db/schema.sql only as needed to keep fresh-database
initialization consistent; both sites must remain compatible with
Migrator.run().

[coderabbitai]

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
rg -n -C3 'processContract|MonitorCycle|thresholdsCrossed|alertsResolved|enabled' tests src/core

Repository: AbdulmalikAlayande/sorokeep

Length of output: 50384

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== candidate files =="
fd -a 'lifecycle|monitor|alerts\.' tests src core | sed 's#^\./##' | sort

echo "== alerts lifecycle test outline/sections =="
wc -l tests/alerts/lifecycle.test.ts
sed -n '1,240p' tests/alerts/lifecycle.test.ts

echo "== monitor processContract relevant implementation =="
sed -n '140,250p' src/core/monitor.ts

echo "== alert repository enabled fields/usages =="
rg -n -C2 'alert_configs|enabled|getAlertConfigsForContract|insertAlertConfig|resolveAlerts|hasUnresolvedAlert' src/db tests/db tests/alerts/lifecycle.test.ts

Repository: AbdulmalikAlayande/sorokeep

Length of output: 50383

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== monitor.test.ts focused grep =="
rg -n -C3 'disabled|enable|thresholdsCrossed|alertsResolved|runMonitorCycle|insertAlertConfig|setAlertConfigEnabled' tests/core/monitor.test.ts tests/alerts/lifecycle.test.ts

echo "== monitor lifecycle tests =="
sed -n '1,260p' tests/core/monitor.test.ts
sed -n '380,516p' tests/core/monitor.test.ts

echo "== alerts lifecycle tail =="
sed -n '480,516p' tests/alerts/lifecycle.test.ts

Repository: AbdulmalikAlayande/sorokeep

Length of output: 50383

---

Add monitor-cycle coverage for enabled state.

CLI coverage is fine, but add an E2E case in tests/core/monitor.test.ts or tests/alerts/lifecycle.test.ts using setAlertConfigEnabled/enabled: 0 to prove disabled configs do not increment thresholdsCrossed or resolve alerts, and that re-enabling lets the next eligible cycle fire.

🤖 Prompt for 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.

In `@tests/commands/alerts.test.ts` around lines 542 - 607, Add an E2E
monitor-cycle test in tests/core/monitor.test.ts or
tests/alerts/lifecycle.test.ts covering setAlertConfigEnabled and the enabled: 0
state. Verify a disabled config neither increments thresholdsCrossed nor
resolves alerts, then re-enable it and confirm the next eligible monitor cycle
fires the alert.

[coderabbitai]

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Remove the duplicated enable/disable suite.

This block repeats lines 542-607 case-for-case; retain one suite to avoid duplicated maintenance and divergent assertions.

🤖 Prompt for 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.

In `@tests/commands/alerts.test.ts` around lines 653 - 724, Remove the duplicated
“alerts enable / disable” describe suite containing the tests for disabling,
re-enabling, missing IDs, and non-numeric IDs; retain the other identical suite
around the existing alerts command tests so each behavior remains covered
exactly once.

[gitguardian]

⚠️ GitGuardian has uncovered 1 secret following the scan of your pull request.

Please consider investigating the findings and remediating the incidents. Failure to do so may lead to compromising the associated services or software components.

Since your pull request originates from a forked repository, GitGuardian is not able to associate the secrets uncovered with secret incidents on your GitGuardian dashboard.
Skipping this check run and merging your pull request will create secret incidents on your GitGuardian dashboard.

🔎 Detected hardcoded secret in your pull request

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

1. Understand the implications of revoking this secret by investigating where it is used in your code.
2. Replace and store your secret safely. Learn here the best practices.
3. Revoke and rotate this secret.
4. 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.}

[AbdulmalikAlayande]

Merged into main (ffc36fc) — thanks, the enabled-column design is exactly right.

A few fixes were needed before this was safe to ship:

• Migration file collided with an already-merged migration number (same class of issue as a couple of others this wave) — renamed 002→005.
• The test file had a corrupted describe block: an entire 'alerts enable / disable' test suite had gotten pasted into the middle of the 'alerts remove' test, orphaning code outside any it() — this wouldn't have compiled. Restored it and kept the separately-written, correctly-scoped enable/disable tests that already existed later in the file.
• Added monitor-level tests for the actual acceptance criteria (disabled configs don't fire, re-enabling resumes detection) — the existing tests only checked the CLI toggle flips the DB flag, not that monitor.ts actually respects it.

Verified locally: tsc clean, full suite 1210/1210, npm audit clean, build succeeds, manually smoke-tested add → disable → enable end-to-end. Thanks for this one!