[glm-grade=A] fix(notifications): un-gate 7 dead emitters + repair legacy 4-arg send shape (DC-094)
Two silent-death defects in one class: 1. Seven emitters absent from DEFAULT events, so the send() gate (config.events[canonical] !== true) dropped them on every install: ssl-cert-expiry, dns-propagation, drift-detected, dependency-restart-complete/-failed, recipeRemoved, workflow. All now default ON; dependency-restart spellings fold onto one canonical toggle; recipeRemoved aliases to recipe-removed in both the manager and route alias maps. 2. Nine call sites used a legacy 4-arg send(event, title, message, type) against the 3-arg signature: the message string landed in the type slot (Discord embed color fell back to info-blue, history.type wrong) and providers received the TITLE as the body - deploy-failure notifications carried no error text at all. Fixed at source (9 sites) plus a type-guarded shim in send() for external legacy callers. Also: explicit data.title now flows to ntfy Title header, email subject, Discord embed title, and history; settings UI gains 9 event toggles (separate Backup Complete/Failed) with defaults-on semantics. Tests: +24 (new DC-094 suite: defaults, gate pass-through, alias folding send-time and load-time, stored-config inheritance, shim body/title/ color/subject/history, type-guard, 3-arg no-regression); 4 assertions in bundled-workflows-health-check updated from the old 4-arg mock contract to the canonical shape (same behavior asserted). Full suite 116 suites / 2706 tests green. Judge: GLM-5.3 cold read via delegate_task (deleg_8a7cedd0), grade A, zero blockers; 2 polish items (Backups toggle conflation, shim type-guard) folded into this commit. Verdict URN: urn:ump:uyipjjwdjqjy3alvceqxvlucrymd7hnsben5udpp2bqycoh3l2la
This commit is contained in:
@@ -15,6 +15,8 @@ const nodemailer = require('nodemailer');
|
||||
// 'auto-restart' (resource-monitor.js) were absent from DEFAULT events and
|
||||
// silently dropped, and UI camelCase toggles never reached the kebab keys the
|
||||
// gate reads — the toggles were cosmetic.
|
||||
// DC-094: recipe emitters used 'recipeRemoved' (camelCase) — alias onto the
|
||||
// canonical kebab key like every other spelling drift before it.
|
||||
const EVENT_ALIASES = {
|
||||
containerDown: 'container-down',
|
||||
containerUp: 'container-up',
|
||||
@@ -27,6 +29,11 @@ const EVENT_ALIASES = {
|
||||
backupComplete: 'backup-complete',
|
||||
backupFailed: 'backup-failed',
|
||||
autoRestart: 'auto-restart',
|
||||
recipeRemoved: 'recipe-removed',
|
||||
// DC-094: dependency-manager fires two spellings; one canonical toggle
|
||||
// gates both (stored configs with either key fold onto it at load).
|
||||
'dependency-restart-complete': 'dependency-restart',
|
||||
'dependency-restart-failed': 'dependency-restart',
|
||||
};
|
||||
|
||||
const DEFAULT_CONFIG = {
|
||||
@@ -49,7 +56,23 @@ const DEFAULT_CONFIG = {
|
||||
// silently dropped before this fix.
|
||||
'deploy-success': true,
|
||||
'deploy-failed': true,
|
||||
'auto-restart': true
|
||||
'auto-restart': true,
|
||||
// DC-094: seven more emitters were absent from DEFAULT events, so the
|
||||
// send() gate (config.events[canonical] !== true) silently dropped every
|
||||
// one of them: SSL expiry warnings, DNS propagation results, config
|
||||
// drift alerts, dependency restart results, recipe removals, and
|
||||
// workflow notify actions. All default ON — every one of these fires
|
||||
// only when something actually happened (and workflow notify actions
|
||||
// are explicitly authored by the operator, so an off-by-default gate
|
||||
// would just re-create this same silent-death bug). Recipe DEPLOY
|
||||
// notifications need no key: recipes/deploy.js emits the
|
||||
// deploymentSuccess/deploymentFailed aliases → deploy-success/failed.
|
||||
'ssl-cert-expiry': true,
|
||||
'dns-propagation': true,
|
||||
'drift-detected': true,
|
||||
'dependency-restart': true,
|
||||
'recipe-removed': true,
|
||||
'workflow': true
|
||||
}
|
||||
};
|
||||
|
||||
@@ -189,6 +212,25 @@ class NotificationManager extends EventEmitter {
|
||||
* Send notification via all enabled providers
|
||||
*/
|
||||
async send(event, data, type = 'info') {
|
||||
// DC-094: nine in-repo call sites used a legacy 4-arg shape
|
||||
// send(event, title, message, type) against this 3-arg signature, so the
|
||||
// message string silently landed in the `type` slot (Discord embed color
|
||||
// fell back to info-blue) and providers received the TITLE as the body —
|
||||
// deploy-failure notifications lost the actual error text entirely. The
|
||||
// in-repo sites are fixed at source; this shim stays so any external
|
||||
// caller of the same legacy shape keeps working instead of silently
|
||||
// degrading again. Guarded on typeof data === 'string': the legacy shape
|
||||
// always passed a string title as arg 2, so a hypothetical
|
||||
// send(event, {...}, type, extra) call is left untouched rather than
|
||||
// mangled by the rewrite.
|
||||
if (arguments.length >= 4 && typeof data === 'string') {
|
||||
const legacyTitle = data;
|
||||
const legacyMessage = type;
|
||||
const legacyType = arguments[3];
|
||||
data = { title: legacyTitle, text: legacyMessage };
|
||||
type = legacyType || 'info';
|
||||
}
|
||||
|
||||
if (!this.config.enabled) {
|
||||
return { success: false, error: 'Notifications disabled' };
|
||||
}
|
||||
@@ -204,6 +246,12 @@ class NotificationManager extends EventEmitter {
|
||||
return { success: false, error: `Event ${canonical} not enabled` };
|
||||
}
|
||||
|
||||
// Provider-facing title: an explicit data.title (legacy 4-arg callers
|
||||
// passed a specific one, e.g. "Recipe Deployed") beats the generic
|
||||
// per-event title.
|
||||
const title = (data && typeof data === 'object' && typeof data.title === 'string' && data.title)
|
||||
|| this._formatTitle(canonical);
|
||||
|
||||
const results = [];
|
||||
const providers = this.config.providers;
|
||||
|
||||
@@ -230,7 +278,7 @@ class NotificationManager extends EventEmitter {
|
||||
// ntfy
|
||||
if (providers.ntfy?.enabled && providers.ntfy?.topic) {
|
||||
try {
|
||||
const result = await this.sendNtfy(this._formatText(data, canonical), this._formatTitle(canonical));
|
||||
const result = await this.sendNtfy(this._formatText(data, canonical), title);
|
||||
results.push({ provider: 'ntfy', ...result });
|
||||
} catch (error) {
|
||||
results.push({ provider: 'ntfy', success: false, error: error.message });
|
||||
@@ -241,7 +289,7 @@ class NotificationManager extends EventEmitter {
|
||||
if (providers.email?.enabled && providers.email?.host && providers.email?.to) {
|
||||
try {
|
||||
const result = await this.sendEmail(
|
||||
this._formatTitle(canonical),
|
||||
title,
|
||||
this._formatText(data, canonical)
|
||||
);
|
||||
results.push({ provider: 'email', ...result });
|
||||
@@ -252,7 +300,7 @@ class NotificationManager extends EventEmitter {
|
||||
|
||||
const allSucceeded = results.every(r => r.success);
|
||||
this._addToHistory({
|
||||
title: this._formatTitle(canonical),
|
||||
title,
|
||||
type,
|
||||
event: canonical,
|
||||
results
|
||||
@@ -443,7 +491,14 @@ class NotificationManager extends EventEmitter {
|
||||
'test': 'Test Notification',
|
||||
'auto-restart': 'Auto-Restart',
|
||||
'deploy-success': 'Deployment Success',
|
||||
'deploy-failed': 'Deployment Failed'
|
||||
'deploy-failed': 'Deployment Failed',
|
||||
// DC-094: newly gated events get real provider titles too.
|
||||
'ssl-cert-expiry': 'SSL Certificate Expiry',
|
||||
'dns-propagation': 'DNS Propagation',
|
||||
'drift-detected': 'Configuration Drift',
|
||||
'dependency-restart': 'Dependency Restart',
|
||||
'recipe-removed': 'Recipe Removed',
|
||||
'workflow': 'Workflow Notification'
|
||||
};
|
||||
return titles[event] || 'DashCaddy Notification';
|
||||
}
|
||||
@@ -458,7 +513,7 @@ class NotificationManager extends EventEmitter {
|
||||
if (data.embed) return data.embed;
|
||||
|
||||
return {
|
||||
title: this._formatTitle(event),
|
||||
title: (typeof data.title === 'string' && data.title) || this._formatTitle(event),
|
||||
description: data.text || data.message || '',
|
||||
color: this._getTypeColor(type),
|
||||
timestamp: new Date().toISOString()
|
||||
|
||||
@@ -500,7 +500,10 @@ class WorkflowEngine extends EventEmitter {
|
||||
}
|
||||
|
||||
log.info('workflow', 'Sending notification', { message });
|
||||
notification.send('workflow', 'Workflow Notification', message, 'info');
|
||||
notification.send('workflow', {
|
||||
title: 'Workflow Notification',
|
||||
text: message
|
||||
}, 'info');
|
||||
|
||||
return { notified: true, message };
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user