fix: auto-create notification channel tables on startup#82
Conversation
The webhook feature returned "Notification channels are not available yet" because the notification_channels, delivery_log, and slack_connections tables did not exist in the database. This adds CREATE TABLE IF NOT EXISTS statements during app startup, following the same pattern used for the api_keys table. https://claude.ai/code/session_011SJrk4Nqa62f3QE4WTqr9p
Tests verify DDL structure for notification_channels, delivery_log, and slack_connections tables, error logging on creation failure, and that channel routes are still registered when table creation fails. https://claude.ai/code/session_011SJrk4Nqa62f3QE4WTqr9p
Adds a PostgreSQL CHECK constraint ensuring bot_token matches the AES-256-GCM ciphertext format (base64:base64:base64), preventing accidental plaintext storage. Also adds a SQL comment on the config JSONB column noting it may contain webhook secrets. https://claude.ai/code/session_011SJrk4Nqa62f3QE4WTqr9p
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds startup initialization in Changes
Sequence Diagram(s)sequenceDiagram
participant Startup as Server Startup
participant DB as Database
participant Logger as Logger
participant Router as Route Registrar
Startup->>DB: Execute DDL (notification_channels, delivery_log, slack_connections)
DB-->>Startup: Success / Error
alt Success
Startup->>Logger: Log DDL success (info)
else Error
Startup->>Logger: Log DDL failure (non-fatal)
end
Startup->>Router: Register routes (notification routes guarded at runtime)
Router-->>Startup: Routes registered
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@server/routes.migration.test.ts`:
- Around line 342-344: The test currently only checks that the CREATE string
contains "CHECK" and "scope"; update the assertion that inspects scCreate to
validate the full CHECK constraint regex for encrypted ciphertext (e.g., the
base64:base64:base64 pattern) by asserting scCreate contains the expected
pattern string or a regexp such as
"CHECK.*'\\^[A-Za-z0-9+/=]+:[A-Za-z0-9+/=]+:[A-Za-z0-9+/=]+\\$'"; locate the
scCreate variable in the test and replace or add an expectation that matches
this exact constraint string/regexp to ensure the encryption format constraint
is enforced.
In `@server/routes.ts`:
- Line 135: The bot_token CHECK constraint currently allows very short base64
parts (e.g., "a:b:c"); update the CHECK regex on the bot_token column to enforce
minimum base64 lengths for each colon-separated part (e.g., IV >=16 chars,
ciphertext >=20 chars, auth tag >=22 chars) so the pattern requires at least
those character counts while still matching base64 characters and colons; modify
the CHECK expression that references bot_token to use the stricter regex so
stored values must meet the new minimum-lengths for IV, ciphertext, and auth
tag.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4c5283ea-e218-4eb1-a16f-0d4017682649
📒 Files selected for processing (2)
server/routes.migration.test.tsserver/routes.ts
… lengths The CHECK regex previously accepted trivially short values like "a:b:c". Now enforces minimum base64 lengths matching AES-256-GCM output sizes (IV >=16, ciphertext >=20, auth tag >=22 chars). Also strengthens the test assertion to verify the actual regex pattern, not just the CHECK keyword. https://claude.ai/code/session_011SJrk4Nqa62f3QE4WTqr9p
Instead of showing a scary "Slack integration is not available" error to end users (who cannot fix server configuration), hide the entire Slack section when the backend reports available=false. This stops the recurring cycle of "improving" the error message (#82, #83, #85). https://claude.ai/code/session_017WdTqmNUwoVYvsgEDoLrHK
Summary
Notification channel routes (
/api/monitors/:id/channels) return 503 "not available yet" when thenotification_channels,delivery_log, andslack_connectionstables don't exist — which happens on fresh deployments whereschema:pushhasn't been run after these tables were added to the Drizzle schema. This PR addsCREATE TABLE IF NOT EXISTSmigrations at server startup (same pattern used forerror_logsandapi_keys), so the tables are always available.Changes
Server startup migration (
server/routes.ts)notification_channelstable with monitor FK, JSONB config column, and indexes (monitor lookup + unique monitor+channel)delivery_logtable with monitor and change FKs, status tracking, and composite indexslack_connectionstable with unique user FK, team metadata, and aCHECKconstraint onbot_tokenenforcing the AES-256-GCM encrypted ciphertext format (base64:base64:base64) to prevent accidental plaintext storageconfig JSONBcolumn noting it may contain webhook secretsTests (
server/routes.migration.test.ts)notification_channels,delivery_log,slack_connectionsDDL is issuedHow to test
npm run test— all 1025 tests should passschema:push) and verify/api/monitors/:id/channelsreturns 200 (not 503)slack_connectionsrejects plaintextbot_tokenvalues by attempting a direct INSERT — should fail the CHECK constrainthttps://claude.ai/code/session_011SJrk4Nqa62f3QE4WTqr9p
Summary by CodeRabbit
New Features
Bug Fixes
Tests