mirror of
https://github.com/Studio-Saelix/sencho.git
synced 2026-07-26 11:49:16 +00:00
fix: fail closed on corrupt mute schedules and clean invalid replicas
Empty or whitespace stored schedules no longer act as all-day mutes. Invalid schedules trigger remote DELETE cleanup, and the weekly-window form gains accessibility attributes plus component coverage.
This commit is contained in:
@@ -103,5 +103,12 @@ describe('scheduleAllowsSuppression / stored parse', () => {
|
||||
expect(scheduleAllowsSuppression(null, true, Date.now())).toBe(false);
|
||||
expect(parseStoredNotificationSchedule('{not-json')).toEqual({ kind: 'invalid' });
|
||||
expect(parseStoredNotificationSchedule(null)).toEqual({ kind: 'null' });
|
||||
expect(parseStoredNotificationSchedule(undefined)).toEqual({ kind: 'null' });
|
||||
});
|
||||
|
||||
it('treats empty and whitespace strings as invalid, not legacy null', () => {
|
||||
expect(parseStoredNotificationSchedule('')).toEqual({ kind: 'invalid' });
|
||||
expect(parseStoredNotificationSchedule(' ')).toEqual({ kind: 'invalid' });
|
||||
expect(parseStoredNotificationSchedule('null')).toEqual({ kind: 'invalid' });
|
||||
});
|
||||
});
|
||||
|
||||
@@ -7,6 +7,7 @@ import os from 'os';
|
||||
import path from 'path';
|
||||
import Database from 'better-sqlite3';
|
||||
import { DatabaseService } from '../services/DatabaseService';
|
||||
import { scheduleAllowsSuppression } from '../helpers/notificationSchedule';
|
||||
|
||||
function resetDatabaseSingleton(): void {
|
||||
const holder = DatabaseService as unknown as { instance?: DatabaseService };
|
||||
@@ -89,4 +90,46 @@ describe('notification suppression schedule column migration', () => {
|
||||
const cols2 = db2.getDb().prepare('PRAGMA table_info(notification_suppression_rules)').all() as Array<{ name: string }>;
|
||||
expect(cols2.filter((c) => c.name === 'schedule')).toHaveLength(1);
|
||||
});
|
||||
|
||||
it('empty-string schedule column is invalid and does not suppress via enabled load', { timeout: 60_000 }, () => {
|
||||
scratchDir = fs.mkdtempSync(path.join(os.tmpdir(), 'sencho-supp-sched-empty-'));
|
||||
const dbPath = path.join(scratchDir, 'sencho.db');
|
||||
const seed = new Database(dbPath);
|
||||
try {
|
||||
seed.exec(`
|
||||
CREATE TABLE notification_suppression_rules (
|
||||
id INTEGER PRIMARY KEY AUTOINCREMENT,
|
||||
name TEXT NOT NULL,
|
||||
node_id INTEGER NULL,
|
||||
stack_patterns TEXT NOT NULL,
|
||||
label_ids TEXT NULL,
|
||||
categories TEXT NULL,
|
||||
levels TEXT NULL,
|
||||
applies_to TEXT NOT NULL,
|
||||
enabled INTEGER DEFAULT 1,
|
||||
expires_at INTEGER NULL,
|
||||
schedule TEXT NULL,
|
||||
created_at INTEGER NOT NULL,
|
||||
updated_at INTEGER NOT NULL
|
||||
);
|
||||
INSERT INTO notification_suppression_rules
|
||||
(name, node_id, stack_patterns, label_ids, categories, levels, applies_to, enabled, expires_at, schedule, created_at, updated_at)
|
||||
VALUES ('Empty schedule mute', NULL, '[]', NULL, '["monitor_alert"]', NULL, 'both', 1, NULL, '', 1, 1);
|
||||
`);
|
||||
} finally {
|
||||
seed.close();
|
||||
}
|
||||
|
||||
prevDataDir = process.env.DATA_DIR;
|
||||
process.env.DATA_DIR = scratchDir;
|
||||
resetDatabaseSingleton();
|
||||
const db = DatabaseService.getInstance();
|
||||
|
||||
const rule = db.getEnabledNotificationSuppressionRules().find((r) => r.name === 'Empty schedule mute');
|
||||
expect(rule).toBeDefined();
|
||||
expect(rule!.schedule).toBeNull();
|
||||
expect(rule!.scheduleInvalid).toBe(true);
|
||||
|
||||
expect(scheduleAllowsSuppression(rule!.schedule, rule!.scheduleInvalid, Date.now())).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -155,16 +155,67 @@ describe('notificationSuppressionSync', () => {
|
||||
expect(error.mock.calls.some((c) => String(c[0]).includes('cleanup pending'))).toBe(true);
|
||||
});
|
||||
|
||||
it('skips sync when scheduleInvalid (does not POST as unscheduled)', async () => {
|
||||
it('scheduleInvalid: DELETE success, no POST', async () => {
|
||||
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
|
||||
const error = vi.spyOn(console, 'error').mockImplementation(() => {});
|
||||
mockFetch.mockResolvedValue({ ok: true, status: 200, text: async () => '' });
|
||||
|
||||
syncSuppressionRuleToFleet(makeRule({
|
||||
node_id: 10,
|
||||
schedule: null,
|
||||
scheduleInvalid: true,
|
||||
}));
|
||||
await vi.waitFor(() => expect(warn).toHaveBeenCalled());
|
||||
await vi.waitFor(() => {
|
||||
expect(warn.mock.calls.some((c) => String(c[0]).includes('replica removed'))).toBe(true);
|
||||
});
|
||||
|
||||
expect(mockFetch.mock.calls.every((c) => (c[1] as { method: string }).method === 'DELETE')).toBe(true);
|
||||
expect(error.mock.calls.some((c) => String(c[0]).includes('cleanup pending'))).toBe(false);
|
||||
});
|
||||
|
||||
it('scheduleInvalid: DELETE 404 counts as cleanup success, no POST', async () => {
|
||||
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
|
||||
mockFetch.mockResolvedValue({ ok: false, status: 404, text: async () => 'gone' });
|
||||
|
||||
syncSuppressionRuleToFleet(makeRule({
|
||||
node_id: 10,
|
||||
schedule: null,
|
||||
scheduleInvalid: true,
|
||||
}));
|
||||
await vi.waitFor(() => {
|
||||
expect(warn.mock.calls.some((c) => String(c[0]).includes('replica removed'))).toBe(true);
|
||||
});
|
||||
expect(mockFetch.mock.calls.every((c) => (c[1] as { method: string }).method === 'DELETE')).toBe(true);
|
||||
});
|
||||
|
||||
it('scheduleInvalid: DELETE failure logs pending cleanup, no POST', async () => {
|
||||
const error = vi.spyOn(console, 'error').mockImplementation(() => {});
|
||||
mockFetch.mockResolvedValue({ ok: false, status: 503, text: async () => 'down' });
|
||||
|
||||
syncSuppressionRuleToFleet(makeRule({
|
||||
node_id: 10,
|
||||
schedule: null,
|
||||
scheduleInvalid: true,
|
||||
}));
|
||||
await vi.waitFor(() => expect(error).toHaveBeenCalled());
|
||||
expect(mockFetch.mock.calls.every((c) => (c[1] as { method: string }).method === 'DELETE')).toBe(true);
|
||||
expect(error.mock.calls.some((c) => String(c[0]).includes('cleanup pending'))).toBe(true);
|
||||
});
|
||||
|
||||
it('scheduleInvalid: no proxy target logs pending, no POST', async () => {
|
||||
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
|
||||
const error = vi.spyOn(console, 'error').mockImplementation(() => {});
|
||||
mockGetProxyTarget.mockReturnValue(null);
|
||||
|
||||
syncSuppressionRuleToFleet(makeRule({
|
||||
node_id: 10,
|
||||
schedule: null,
|
||||
scheduleInvalid: true,
|
||||
}));
|
||||
await vi.waitFor(() => expect(error).toHaveBeenCalled());
|
||||
expect(mockFetch).not.toHaveBeenCalled();
|
||||
expect(warn.mock.calls.some((c) => String(c[0]).includes('corrupt schedule'))).toBe(true);
|
||||
expect(warn.mock.calls.some((c) => String(c[0]).includes('replica removed'))).toBe(false);
|
||||
expect(error.mock.calls.some((c) => String(c[0]).includes('cleanup pending'))).toBe(true);
|
||||
});
|
||||
|
||||
it('unscheduled-to-scheduled on unsupported target attempts DELETE', async () => {
|
||||
|
||||
@@ -73,22 +73,29 @@ export function parseNotificationSchedule(raw: unknown): ParseNotificationSchedu
|
||||
}
|
||||
|
||||
/**
|
||||
* Parse a DB column value. Missing/null → legacy null (always in window).
|
||||
* Non-null corrupt JSON → invalid (caller must not suppress).
|
||||
* Parse a DB column value. Only SQL null / undefined → legacy null (always in window).
|
||||
* Empty string, whitespace, or any non-null corrupt value → invalid (must not suppress).
|
||||
*/
|
||||
export function parseStoredNotificationSchedule(
|
||||
raw: unknown,
|
||||
): { kind: 'null' } | { kind: 'ok'; schedule: NotificationSchedule } | { kind: 'invalid' } {
|
||||
if (raw == null || raw === '') return { kind: 'null' };
|
||||
let parsed: unknown = raw;
|
||||
if (raw == null) return { kind: 'null' };
|
||||
if (typeof raw === 'string') {
|
||||
const trimmed = raw.trim();
|
||||
if (trimmed === '') return { kind: 'invalid' };
|
||||
let parsed: unknown;
|
||||
try {
|
||||
parsed = JSON.parse(raw);
|
||||
parsed = JSON.parse(trimmed);
|
||||
} catch {
|
||||
return { kind: 'invalid' };
|
||||
}
|
||||
// JSON null is not SQL NULL; treat as corrupt rather than legacy always-on.
|
||||
if (parsed == null) return { kind: 'invalid' };
|
||||
const result = parseNotificationSchedule(parsed);
|
||||
if (!result.ok) return { kind: 'invalid' };
|
||||
return { kind: 'ok', schedule: result.schedule };
|
||||
}
|
||||
const result = parseNotificationSchedule(parsed);
|
||||
const result = parseNotificationSchedule(raw);
|
||||
if (!result.ok) return { kind: 'invalid' };
|
||||
return { kind: 'ok', schedule: result.schedule };
|
||||
}
|
||||
|
||||
@@ -102,11 +102,25 @@ async function pushOrCleanupScheduled(node: Node, rule: NotificationSuppressionR
|
||||
}
|
||||
}
|
||||
|
||||
async function cleanupInvalidScheduleReplica(node: Node, rule: NotificationSuppressionRule): Promise<void> {
|
||||
// Never POST an invalid schedule (would mute all day on remotes that ignore the field).
|
||||
// Attempt DELETE so a prior valid/unscheduled replica cannot keep muting.
|
||||
try {
|
||||
await deleteRuleOnNode(node, rule.id);
|
||||
console.warn(
|
||||
`[SuppressionSync] Corrupt schedule on rule ${rule.id}: replica removed on node "${node.name}" (id=${node.id}); not posting`,
|
||||
);
|
||||
} catch (err) {
|
||||
console.error(
|
||||
`[SuppressionSync] Corrupt schedule on rule ${rule.id}: cleanup pending on node "${node.name}" (id=${node.id}); ` +
|
||||
`DELETE failed (${getErrorMessage(err, String(err))}). Prior replica may remain until connectivity returns`,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
async function syncRuleToNode(node: Node, rule: NotificationSuppressionRule): Promise<void> {
|
||||
if (rule.scheduleInvalid) {
|
||||
console.warn(
|
||||
`[SuppressionSync] Skipping push of rule ${rule.id} to node "${node.name}": corrupt schedule; not syncing as unscheduled`,
|
||||
);
|
||||
await cleanupInvalidScheduleReplica(node, rule);
|
||||
return;
|
||||
}
|
||||
if (rule.schedule != null) {
|
||||
|
||||
@@ -583,6 +583,8 @@ export function NotificationSuppressionSection({
|
||||
size="sm"
|
||||
variant={selected ? 'default' : 'outline'}
|
||||
className="h-7 px-2 text-xs"
|
||||
aria-pressed={selected}
|
||||
aria-label={label}
|
||||
onClick={() => {
|
||||
setFormScheduleDays((prev) =>
|
||||
selected
|
||||
@@ -598,12 +600,22 @@ export function NotificationSuppressionSection({
|
||||
</div>
|
||||
<div className="grid grid-cols-2 gap-2">
|
||||
<div className="space-y-1">
|
||||
<Label className="text-xs text-muted-foreground">Start (UTC)</Label>
|
||||
<Input type="time" value={formScheduleStart} onChange={(e) => setFormScheduleStart(e.target.value)} />
|
||||
<Label htmlFor="mute-schedule-start" className="text-xs text-muted-foreground">Start (UTC)</Label>
|
||||
<Input
|
||||
id="mute-schedule-start"
|
||||
type="time"
|
||||
value={formScheduleStart}
|
||||
onChange={(e) => setFormScheduleStart(e.target.value)}
|
||||
/>
|
||||
</div>
|
||||
<div className="space-y-1">
|
||||
<Label className="text-xs text-muted-foreground">End (UTC)</Label>
|
||||
<Input type="time" value={formScheduleEnd} onChange={(e) => setFormScheduleEnd(e.target.value)} />
|
||||
<Label htmlFor="mute-schedule-end" className="text-xs text-muted-foreground">End (UTC)</Label>
|
||||
<Input
|
||||
id="mute-schedule-end"
|
||||
type="time"
|
||||
value={formScheduleEnd}
|
||||
onChange={(e) => setFormScheduleEnd(e.target.value)}
|
||||
/>
|
||||
</div>
|
||||
</div>
|
||||
<p className="text-xs text-muted-foreground">
|
||||
|
||||
+153
-13
@@ -1,8 +1,8 @@
|
||||
/**
|
||||
* NotificationSuppressionSection stack pattern chips.
|
||||
* NotificationSuppressionSection stack pattern chips and weekly schedule UI.
|
||||
*/
|
||||
import { describe, it, expect, vi, beforeEach } from 'vitest';
|
||||
import { render, screen, waitFor } from '@testing-library/react';
|
||||
import { fireEvent, render, screen, waitFor } from '@testing-library/react';
|
||||
import userEvent from '@testing-library/user-event';
|
||||
|
||||
vi.mock('@/lib/api', () => ({ apiFetch: vi.fn() }));
|
||||
@@ -38,10 +38,49 @@ vi.mock('@/lib/muteRules', () => ({
|
||||
}));
|
||||
|
||||
import { apiFetch } from '@/lib/api';
|
||||
import { toast } from '@/components/ui/toast-store';
|
||||
import { NotificationSuppressionSection } from '../NotificationSuppressionSection';
|
||||
|
||||
const mockedFetch = apiFetch as unknown as ReturnType<typeof vi.fn>;
|
||||
|
||||
const scheduledRule = {
|
||||
id: 42,
|
||||
name: 'Weekend mute',
|
||||
node_id: null,
|
||||
stack_patterns: ['prod-*'],
|
||||
label_ids: null,
|
||||
categories: null,
|
||||
levels: null,
|
||||
applies_to: 'both',
|
||||
enabled: true,
|
||||
expires_at: null,
|
||||
schedule: {
|
||||
days: [6],
|
||||
start_minute: 120,
|
||||
end_minute: 360,
|
||||
tz: 'UTC',
|
||||
},
|
||||
created_at: 1,
|
||||
updated_at: 1,
|
||||
};
|
||||
|
||||
function mockListRules(rules: unknown[] = []) {
|
||||
mockedFetch.mockImplementation(async (url: string, opts?: { method?: string }) => {
|
||||
if (url === '/notification-suppression-rules' && !opts?.method) {
|
||||
return { ok: true, json: async () => rules };
|
||||
}
|
||||
if (url === '/stacks') return { ok: true, json: async () => ['staging'] };
|
||||
if (url === '/labels') return { ok: true, json: async () => [] };
|
||||
if (url === '/notification-suppression-rules' && opts?.method === 'POST') {
|
||||
return { ok: true, json: async () => ({ id: 1 }) };
|
||||
}
|
||||
if (typeof url === 'string' && url.startsWith('/notification-suppression-rules/') && opts?.method === 'PUT') {
|
||||
return { ok: true, json: async () => ({ id: 42 }) };
|
||||
}
|
||||
return { ok: true, json: async () => ([]) };
|
||||
});
|
||||
}
|
||||
|
||||
async function openMuteForm() {
|
||||
render(<NotificationSuppressionSection />);
|
||||
await waitFor(() => expect(screen.getByRole('button', { name: /Add mute rule|Add rule/i })).toBeInTheDocument());
|
||||
@@ -49,20 +88,16 @@ async function openMuteForm() {
|
||||
await waitFor(() => expect(screen.getByRole('dialog', { name: /New mute rule/i })).toBeInTheDocument());
|
||||
}
|
||||
|
||||
async function enableWeeklyWindow() {
|
||||
await userEvent.click(screen.getByLabelText(/Weekly window \(UTC\)/i));
|
||||
await waitFor(() => expect(screen.getByRole('button', { name: 'Sat' })).toBeInTheDocument());
|
||||
}
|
||||
|
||||
describe('NotificationSuppressionSection', () => {
|
||||
beforeEach(() => {
|
||||
mockedFetch.mockReset();
|
||||
mockedFetch.mockImplementation(async (url: string, opts?: { method?: string }) => {
|
||||
if (url === '/notification-suppression-rules' && !opts?.method) {
|
||||
return { ok: true, json: async () => [] };
|
||||
}
|
||||
if (url === '/stacks') return { ok: true, json: async () => ['staging'] };
|
||||
if (url === '/labels') return { ok: true, json: async () => [] };
|
||||
if (url === '/notification-suppression-rules' && opts?.method === 'POST') {
|
||||
return { ok: true, json: async () => ({ id: 1 }) };
|
||||
}
|
||||
return { ok: true, json: async () => ([]) };
|
||||
});
|
||||
vi.mocked(toast.error).mockClear();
|
||||
mockListRules([]);
|
||||
});
|
||||
|
||||
it('posts normalized stack patterns and null levels', async () => {
|
||||
@@ -129,4 +164,109 @@ describe('NotificationSuppressionSection', () => {
|
||||
expect(posts).toHaveLength(0);
|
||||
});
|
||||
});
|
||||
|
||||
it('posts a weekly UTC schedule with accessible day and time controls', async () => {
|
||||
await openMuteForm();
|
||||
|
||||
await userEvent.type(screen.getByPlaceholderText(/Mute staging/i), 'Sat window');
|
||||
await userEvent.type(screen.getByPlaceholderText(/Type a pattern/i), 'prod-*{Enter}');
|
||||
await enableWeeklyWindow();
|
||||
|
||||
const sat = screen.getByRole('button', { name: 'Sat' });
|
||||
expect(sat).toHaveAttribute('aria-pressed', 'false');
|
||||
await userEvent.click(sat);
|
||||
expect(sat).toHaveAttribute('aria-pressed', 'true');
|
||||
|
||||
expect(screen.getByLabelText('Start (UTC)')).toHaveAttribute('id', 'mute-schedule-start');
|
||||
expect(screen.getByLabelText('End (UTC)')).toHaveAttribute('id', 'mute-schedule-end');
|
||||
fireEvent.change(screen.getByLabelText('Start (UTC)'), { target: { value: '02:00' } });
|
||||
fireEvent.change(screen.getByLabelText('End (UTC)'), { target: { value: '06:00' } });
|
||||
|
||||
await userEvent.click(screen.getByRole('button', { name: /Create|Update/i }));
|
||||
await waitFor(() => {
|
||||
const post = mockedFetch.mock.calls.find(
|
||||
([url, opts]) => url === '/notification-suppression-rules' && (opts as { method?: string })?.method === 'POST',
|
||||
);
|
||||
expect(post).toBeTruthy();
|
||||
const body = JSON.parse((post![1] as { body: string }).body);
|
||||
expect(body.schedule).toEqual({
|
||||
days: [6],
|
||||
start_minute: 120,
|
||||
end_minute: 360,
|
||||
tz: 'UTC',
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
it('blocks scheduled create when no weekday is selected', async () => {
|
||||
await openMuteForm();
|
||||
|
||||
await userEvent.type(screen.getByPlaceholderText(/Mute staging/i), 'No days');
|
||||
await userEvent.type(screen.getByPlaceholderText(/Type a pattern/i), 'prod-*{Enter}');
|
||||
await enableWeeklyWindow();
|
||||
await userEvent.click(screen.getByRole('button', { name: /Create|Update/i }));
|
||||
|
||||
await waitFor(() => {
|
||||
expect(toast.error).toHaveBeenCalledWith('Select at least one day for the weekly window.');
|
||||
});
|
||||
const posts = mockedFetch.mock.calls.filter(
|
||||
([url, opts]) => url === '/notification-suppression-rules' && (opts as { method?: string })?.method === 'POST',
|
||||
);
|
||||
expect(posts).toHaveLength(0);
|
||||
});
|
||||
|
||||
it('hydrates edit form from an existing schedule and keeps it on PUT', async () => {
|
||||
mockListRules([scheduledRule]);
|
||||
render(<NotificationSuppressionSection />);
|
||||
await waitFor(() => expect(screen.getByText('Weekend mute')).toBeInTheDocument());
|
||||
expect(screen.getByText(/UTC Sat 02:00-06:00/)).toBeInTheDocument();
|
||||
|
||||
await userEvent.click(screen.getByTitle('Edit'));
|
||||
await waitFor(() => expect(screen.getByRole('dialog', { name: /Edit mute rule/i })).toBeInTheDocument());
|
||||
|
||||
expect(screen.getByLabelText(/Weekly window \(UTC\)/i)).toHaveAttribute('aria-checked', 'true');
|
||||
expect(screen.getByRole('button', { name: 'Sat' })).toHaveAttribute('aria-pressed', 'true');
|
||||
expect(screen.getByLabelText('Start (UTC)')).toHaveValue('02:00');
|
||||
expect(screen.getByLabelText('End (UTC)')).toHaveValue('06:00');
|
||||
|
||||
await userEvent.click(screen.getByRole('button', { name: /Create|Update/i }));
|
||||
await waitFor(() => {
|
||||
const put = mockedFetch.mock.calls.find(
|
||||
([url, opts]) =>
|
||||
url === '/notification-suppression-rules/42' && (opts as { method?: string })?.method === 'PUT',
|
||||
);
|
||||
expect(put).toBeTruthy();
|
||||
const body = JSON.parse((put![1] as { body: string }).body);
|
||||
expect(body.schedule).toEqual({
|
||||
days: [6],
|
||||
start_minute: 120,
|
||||
end_minute: 360,
|
||||
tz: 'UTC',
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
it('clears schedule to null when weekly window is turned off on edit', async () => {
|
||||
mockListRules([scheduledRule]);
|
||||
render(<NotificationSuppressionSection />);
|
||||
await waitFor(() => expect(screen.getByText('Weekend mute')).toBeInTheDocument());
|
||||
|
||||
await userEvent.click(screen.getByTitle('Edit'));
|
||||
await waitFor(() => expect(screen.getByRole('dialog', { name: /Edit mute rule/i })).toBeInTheDocument());
|
||||
await userEvent.click(screen.getByLabelText(/Weekly window \(UTC\)/i));
|
||||
await waitFor(() => {
|
||||
expect(screen.queryByRole('button', { name: 'Sat' })).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
await userEvent.click(screen.getByRole('button', { name: /Create|Update/i }));
|
||||
await waitFor(() => {
|
||||
const put = mockedFetch.mock.calls.find(
|
||||
([url, opts]) =>
|
||||
url === '/notification-suppression-rules/42' && (opts as { method?: string })?.method === 'PUT',
|
||||
);
|
||||
expect(put).toBeTruthy();
|
||||
const body = JSON.parse((put![1] as { body: string }).body);
|
||||
expect(body.schedule).toBeNull();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user