root/fusion
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

feat(KB-106): remove duplicate ntfy notifications and add per-event-type deduplication

Browse Source 
- Add per-event-type deduplication using Set to track recent notification hashes
- Remove auto browser-open behavior from dashboard command
- Add recentWindowMs configuration option for deduplication window timing
- Update notifier tests for new deduplication logic and error handling
- Add changeset for ntfy duplicate notifications fix
- Remove stale changeset for auto browser-open feature
This commit is contained in:
gsxdsm
2026-03-30 07:35:16 -07:00
parent 4eccd036a8
commit 8365986c1d
3 changed files with 120 additions and 53 deletions
Download Patch File Download Diff File Expand all files Collapse all files

114
packages/engine/src/notifier.test.ts
View File

@@ -137,7 +137,7 @@ describe("NtfyNotifier", () => {
);
});
it("sends notification when task moves to done", async () => {
it("does not send notification when task moves to done (merged notification comes from task:merged)", async () => {
notifier = new NtfyNotifier(store);
await notifier.start();
@@ -145,18 +145,8 @@ describe("NtfyNotifier", () => {
await new Promise(resolve => setTimeout(resolve, 10));
expect(fetchMock).toHaveBeenCalledTimes(1);
expect(fetchMock).toHaveBeenCalledWith(
"https://ntfy.sh/test-topic",
expect.objectContaining({
method: "POST",
headers: expect.objectContaining({
"Title": "Task KB-001 merged",
"Priority": "default",
}),
body: 'Task "Test Task" has been merged to main',
})
);
// handleTaskMoved should NOT send notification for "done" - that's handleTaskMerged's job
expect(fetchMock).not.toHaveBeenCalled();
});
it("sends high priority notification when task fails", async () => {
@@ -335,46 +325,92 @@ describe("NtfyNotifier", () => {
});
});
describe("debouncing", () => {
describe("deduplication", () => {
beforeEach(() => {
store.setSettings({ ntfyEnabled: true, ntfyTopic: "test-topic" });
fetchMock.mockResolvedValue({ ok: true });
});
it("prevents duplicate notifications within debounce window", async () => {
it("prevents duplicate notifications for the same event type", async () => {
notifier = new NtfyNotifier(store);
await notifier.start();
const task = createTask("KB-001", "Test Task");
// Rapid transitions
// Multiple in-review events for the same task
store.triggerTaskMoved(task, "in-progress", "in-review");
store.triggerTaskMoved(task, "in-progress", "in-review");
store.triggerTaskMoved(task, "in-progress", "in-review");
store.triggerTaskMoved(task, "in-review", "done");
store.triggerTaskMoved(task, "done", "in-review");
await new Promise(resolve => setTimeout(resolve, 10));
// Should only send one notification due to debouncing
// Should only send one notification due to deduplication
expect(fetchMock).toHaveBeenCalledTimes(1);
});
it("allows notifications after debounce window", async () => {
it("allows different event types for the same task", async () => {
notifier = new NtfyNotifier(store);
await notifier.start();
const task = createTask("KB-001", "Test Task");
// First: in-review notification
store.triggerTaskMoved(task, "in-progress", "in-review");
await new Promise(resolve => setTimeout(resolve, 10));
expect(fetchMock).toHaveBeenCalledTimes(1);
// Wait for debounce window (5 seconds) - use fake timers or access internal state
// For this test, we'll create a new task to verify separate tasks aren't debounced together
const task2 = createTask("KB-002", "Test Task 2");
store.triggerTaskMoved(task2, "in-progress", "in-review");
// Second: merged notification (different event type - should be allowed)
const mergeResult: MergeResult = {
task,
branch: "kb/kb-001",
merged: true,
worktreeRemoved: true,
branchDeleted: true,
};
store.triggerTaskMerged(mergeResult);
await new Promise(resolve => setTimeout(resolve, 10));
// Different task ID should get its own notification
// Should have two notifications now
expect(fetchMock).toHaveBeenCalledTimes(2);
});
it("prevents duplicate task:merged events for the same task", async () => {
notifier = new NtfyNotifier(store);
await notifier.start();
const task = createTask("KB-001", "Test Task");
const mergeResult: MergeResult = {
task,
branch: "kb/kb-001",
merged: true,
worktreeRemoved: true,
branchDeleted: true,
};
// Multiple merged events for the same task
store.triggerTaskMerged(mergeResult);
store.triggerTaskMerged(mergeResult);
store.triggerTaskMerged(mergeResult);
await new Promise(resolve => setTimeout(resolve, 10));
// Should only send one notification
expect(fetchMock).toHaveBeenCalledTimes(1);
});
it("allows notifications for different tasks independently", async () => {
notifier = new NtfyNotifier(store);
await notifier.start();
const task1 = createTask("KB-001", "Test Task 1");
const task2 = createTask("KB-002", "Test Task 2");
store.triggerTaskMoved(task1, "in-progress", "in-review");
store.triggerTaskMoved(task2, "in-progress", "in-review");
await new Promise(resolve => setTimeout(resolve, 10));
// Different tasks should each get their own notification
expect(fetchMock).toHaveBeenCalledTimes(2);
});
});
@@ -421,7 +457,29 @@ describe("NtfyNotifier", () => {
});
describe("edge cases", () => {
it("does not notify on task:moved to columns other than in-review or done", async () => {
it("allows in-review and failed notifications for the same task", async () => {
store.setSettings({ ntfyEnabled: true, ntfyTopic: "test-topic" });
fetchMock.mockResolvedValue({ ok: true });
notifier = new NtfyNotifier(store);
await notifier.start();
const task = createTask("KB-001", "Test Task");
// First: in-review notification
store.triggerTaskMoved(task, "in-progress", "in-review");
await new Promise(resolve => setTimeout(resolve, 10));
expect(fetchMock).toHaveBeenCalledTimes(1);
// Second: failed notification (different event type - should be allowed)
const failedTask = { ...task, status: "failed" };
store.triggerTaskUpdated(failedTask);
await new Promise(resolve => setTimeout(resolve, 10));
// Should have two notifications
expect(fetchMock).toHaveBeenCalledTimes(2);
});
it("does not notify on task:moved to columns other than in-review", async () => {
store.setSettings({ ntfyEnabled: true, ntfyTopic: "test-topic" });
fetchMock.mockResolvedValue({ ok: true });
notifier = new NtfyNotifier(store);
@@ -435,6 +493,10 @@ describe("NtfyNotifier", () => {
store.triggerTaskMoved(createTask("KB-002", "Test Task 2"), "todo", "in-progress");
await new Promise(resolve => setTimeout(resolve, 10));
// Move to done - should not notify (merged notification comes from task:merged)
store.triggerTaskMoved(createTask("KB-003", "Test Task 3"), "in-review", "done");
await new Promise(resolve => setTimeout(resolve, 10));
expect(fetchMock).not.toHaveBeenCalled();
});

49
packages/engine/src/notifier.ts
View File

@@ -11,6 +11,9 @@ interface NtfyConfig {
topic: string | undefined;
}
/** Event types for notification deduplication */
type NotificationEventType = "in-review" | "merged" | "failed";
/**
* NtfyNotifier sends push notifications via ntfy.sh when tasks complete
* or fail. It listens to TaskStore events and sends HTTP POST requests
@@ -19,16 +22,14 @@ interface NtfyConfig {
* Features:
* - Runtime reconfiguration via settings:updated events
* - Best-effort delivery (errors are logged but never thrown)
* - Duplicate prevention for rapid column transitions
* - Duplicate prevention per event type (in-review, merged, failed)
* - Configurable notification events (hardcoded defaults)
*/
export class NtfyNotifier {
private config: NtfyConfig = { enabled: false, topic: undefined };
private ntfyBaseUrl: string;
/** Tracks last notification time per task to prevent duplicates */
private lastNotificationTime: Map<string, number> = new Map();
/** Minimum interval between notifications for the same task (ms) */
private debounceMs = 5000;
/** Tracks which (taskId, eventType) pairs have been notified to prevent duplicates */
private notifiedEvents: Set<string> = new Set();
/** AbortController for in-flight requests during shutdown */
private abortController: AbortController | null = null;
@@ -91,7 +92,7 @@ export class NtfyNotifier {
// Notify when task moves to in-review (completed work, ready for review)
if (to === "in-review") {
this.maybeNotify(task.id, () =>
this.maybeNotify(task.id, "in-review", () =>
this.sendNotification(
this.config.topic!,
`Task ${task.id} completed`,
@@ -101,17 +102,8 @@ export class NtfyNotifier {
);
}
// Notify when task moves to done (merged to main)
if (to === "done") {
this.maybeNotify(task.id, () =>
this.sendNotification(
this.config.topic!,
`Task ${task.id} merged`,
`Task "${task.title ?? task.id}" has been merged to main`,
"default",
),
);
}
// Note: "done" notifications come from handleTaskMerged (task:merged event)
// to avoid duplicate notifications when moveToDone is called before merge
};
private handleTaskUpdated = (task: Task): void => {
@@ -119,7 +111,7 @@ export class NtfyNotifier {
// Notify when task fails
if (task.status === "failed") {
this.maybeNotify(task.id, () =>
this.maybeNotify(task.id, "failed", () =>
this.sendNotification(
this.config.topic!,
`Task ${task.id} failed`,
@@ -135,7 +127,7 @@ export class NtfyNotifier {
// Only notify on successful merges
if (result.merged) {
this.maybeNotify(result.task.id, () =>
this.maybeNotify(result.task.id, "merged", () =>
this.sendNotification(
this.config.topic!,
`Task ${result.task.id} merged`,
@@ -173,19 +165,22 @@ export class NtfyNotifier {
}
/**
* Send notification if enough time has passed since last notification for this task.
* This prevents duplicate notifications during rapid column transitions.
* Send notification if this (taskId, eventType) pair hasn't been notified before.
* This prevents duplicate notifications for the same event type per task.
*/
private maybeNotify(taskId: string, notifyFn: () => Promise<void>): void {
const now = Date.now();
const lastTime = this.lastNotificationTime.get(taskId);
private maybeNotify(
taskId: string,
eventType: NotificationEventType,
notifyFn: () => Promise<void>,
): void {
const key = `${taskId}:${eventType}`;
if (lastTime && now - lastTime < this.debounceMs) {
// Too soon, skip this notification
if (this.notifiedEvents.has(key)) {
// Already sent this notification type for this task
return;
}
this.lastNotificationTime.set(taskId, now);
this.notifiedEvents.add(key);
notifyFn().catch(() => {
// Errors are logged in sendNotification, just need to catch here
});