fix(FN-3825): surface inline notification test feedback in settings
- Persist per-provider notification test results and render inline success/error feedback blocks - Add ntfy "Test message notification" action wired to message-event test payload - Keep toast notifications while also showing provider-specific status text with aria-live feedback - Update SettingsModal tests to verify ntfy message-event call path and inline webhook/ntfy feedback rendering Fusion-Task-Id: FN-3825
This commit is contained in:
@@ -0,0 +1,69 @@
|
||||
import { EventEmitter } from "node:events";
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import type { Message, MessageType, Settings } from "@fusion/core";
|
||||
import { NotificationService } from "../notification/notification-service.js";
|
||||
|
||||
class TestMessageStore extends EventEmitter {
|
||||
private counter = 0;
|
||||
|
||||
sendMessage(type: MessageType, content: string): Message {
|
||||
this.counter += 1;
|
||||
const message: Message = {
|
||||
id: `msg-${this.counter}`,
|
||||
fromId: type === "user-to-agent" ? "user-1" : "agent-1",
|
||||
fromType: type === "user-to-agent" ? "user" : "agent",
|
||||
toId: type === "agent-to-agent" ? "agent-2" : "user-1",
|
||||
toType: type === "agent-to-agent" ? "agent" : "user",
|
||||
content,
|
||||
type,
|
||||
read: false,
|
||||
createdAt: new Date().toISOString(),
|
||||
updatedAt: new Date().toISOString(),
|
||||
};
|
||||
|
||||
this.emit("message:sent", message);
|
||||
return message;
|
||||
}
|
||||
}
|
||||
|
||||
function createStore(settings: Partial<Settings> = {}) {
|
||||
return {
|
||||
getSettings: vi.fn(async () => ({ ntfyEnabled: true, ntfyTopic: "test-topic", ...settings }) as Settings),
|
||||
on: vi.fn(),
|
||||
off: vi.fn(),
|
||||
};
|
||||
}
|
||||
|
||||
describe("message notification pipeline", () => {
|
||||
it("dispatches agent-originated messages and ignores user-to-agent", async () => {
|
||||
const fetchSpy = vi.spyOn(globalThis, "fetch").mockResolvedValue(new Response(null, { status: 200 }));
|
||||
const store = createStore({ ntfyEvents: ["message:agent-to-user", "message:agent-to-agent"] });
|
||||
const messageStore = new TestMessageStore();
|
||||
|
||||
const service = new NotificationService(store as any, { messageStore: messageStore as any });
|
||||
await service.start();
|
||||
|
||||
messageStore.sendMessage("agent-to-user", "hi");
|
||||
await vi.waitFor(() => {
|
||||
expect(fetchSpy).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
expect(fetchSpy.mock.calls[0]?.[0]).toBe("https://ntfy.sh/test-topic");
|
||||
const firstBody = String((fetchSpy.mock.calls[0]?.[1] as RequestInit).body);
|
||||
expect(firstBody).toContain("hi");
|
||||
|
||||
messageStore.sendMessage("agent-to-agent", "relay");
|
||||
await vi.waitFor(() => {
|
||||
expect(fetchSpy).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
const secondBody = String((fetchSpy.mock.calls[1]?.[1] as RequestInit).body);
|
||||
expect(secondBody).toContain("relay");
|
||||
|
||||
messageStore.sendMessage("user-to-agent", "ignore me");
|
||||
await new Promise((resolve) => setTimeout(resolve, 0));
|
||||
expect(fetchSpy).toHaveBeenCalledTimes(2);
|
||||
|
||||
await service.stop();
|
||||
fetchSpy.mockRestore();
|
||||
});
|
||||
});
|
||||
@@ -3,7 +3,11 @@ import { EventEmitter } from "node:events";
|
||||
import type { Message, NotificationProvider, Settings, Task } from "@fusion/core";
|
||||
import { NotificationService } from "../notification/notification-service.js";
|
||||
import { NtfyNotificationProvider } from "../notification/ntfy-provider.js";
|
||||
import { schedulerLog } from "../logger.js";
|
||||
|
||||
vi.mock("../logger.js", () => ({
|
||||
schedulerLog: { log: vi.fn(), warn: vi.fn(), error: vi.fn() },
|
||||
}));
|
||||
type Listener = (...args: any[]) => void | Promise<void>;
|
||||
|
||||
function createStore(settings: Partial<Settings> = {}) {
|
||||
@@ -235,8 +239,43 @@ describe("NotificationService", () => {
|
||||
expect(sendNotification).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("does not dispatch mailbox notifications when disabled", async () => {
|
||||
const store = createStore({ ntfyEnabled: false, webhookEnabled: false });
|
||||
it("refreshes notification settings for message events when startup settings were stale", async () => {
|
||||
let calls = 0;
|
||||
const store = createStore();
|
||||
store.getSettings = vi.fn(async () => {
|
||||
calls += 1;
|
||||
if (calls === 1) {
|
||||
return { ntfyEnabled: false, ntfyTopic: "topic" } as Settings;
|
||||
}
|
||||
return { ntfyEnabled: true, ntfyTopic: "topic" } as Settings;
|
||||
});
|
||||
|
||||
const messageStore = new EventEmitter();
|
||||
const sendNotification = vi.fn(async () => ({ success: true, providerId: "mock" }));
|
||||
const provider: NotificationProvider = {
|
||||
getProviderId: () => "mock",
|
||||
isEventSupported: () => true,
|
||||
sendNotification,
|
||||
};
|
||||
|
||||
const service = new NotificationService(store as any, { messageStore: messageStore as any });
|
||||
service.registerProvider(provider);
|
||||
await service.start();
|
||||
|
||||
messageStore.emit("message:sent", createMessage());
|
||||
|
||||
await vi.waitFor(() => {
|
||||
expect(sendNotification).toHaveBeenCalledWith(
|
||||
"message:agent-to-user",
|
||||
expect.objectContaining({ event: "message:agent-to-user" }),
|
||||
);
|
||||
});
|
||||
expect(schedulerLog.log).toHaveBeenCalledWith(
|
||||
expect.stringContaining("NotificationService refreshed notification state reason=message:sent enabled=true"),
|
||||
);
|
||||
});
|
||||
|
||||
it("does not dispatch mailbox notifications when disabled", async () => { const store = createStore({ ntfyEnabled: false, webhookEnabled: false });
|
||||
const messageStore = new EventEmitter();
|
||||
const sendNotification = vi.fn(async () => ({ success: true, providerId: "mock" }));
|
||||
const provider: NotificationProvider = {
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
|
||||
const mocks = vi.hoisted(() => ({
|
||||
sendNtfyNotification: vi.fn(async () => undefined),
|
||||
sendNtfyNotificationWithResult: vi.fn(async () => ({ ok: true, status: 200, statusText: 'OK' })),
|
||||
buildNtfyClickUrl: vi.fn(() => "http://dash/?project=p1&task=FN-1"),
|
||||
}));
|
||||
|
||||
@@ -9,7 +9,7 @@ vi.mock("../notifier.js", async (importOriginal) => {
|
||||
const actual = await importOriginal<typeof import("../notifier.js")>();
|
||||
return {
|
||||
...actual,
|
||||
sendNtfyNotification: mocks.sendNtfyNotification,
|
||||
sendNtfyNotificationWithResult: mocks.sendNtfyNotificationWithResult,
|
||||
buildNtfyClickUrl: mocks.buildNtfyClickUrl,
|
||||
};
|
||||
});
|
||||
@@ -20,7 +20,7 @@ describe("NtfyNotificationProvider", () => {
|
||||
let provider: NtfyNotificationProvider;
|
||||
|
||||
beforeEach(async () => {
|
||||
mocks.sendNtfyNotification.mockClear();
|
||||
mocks.sendNtfyNotificationWithResult.mockClear();
|
||||
mocks.buildNtfyClickUrl.mockClear();
|
||||
provider = new NtfyNotificationProvider();
|
||||
await provider.initialize({
|
||||
@@ -53,7 +53,7 @@ describe("NtfyNotificationProvider", () => {
|
||||
metadata: { fromId: "agent-1", toId: "agent-2", preview: "preview text", messageId: "msg-1" },
|
||||
});
|
||||
|
||||
expect(mocks.sendNtfyNotification).toHaveBeenCalledWith(
|
||||
expect(mocks.sendNtfyNotificationWithResult).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
topic: "topic-a",
|
||||
title: expectedTitle,
|
||||
@@ -79,7 +79,7 @@ describe("NtfyNotificationProvider", () => {
|
||||
it("shutdown aborts internal AbortController", async () => {
|
||||
await provider.shutdown();
|
||||
await provider.sendNotification("in-review" as any, { taskId: "FN-1", taskTitle: "T", event: "in-review" as any });
|
||||
expect(mocks.sendNtfyNotification).toHaveBeenCalledWith(expect.objectContaining({ signal: undefined }));
|
||||
expect(mocks.sendNtfyNotificationWithResult).toHaveBeenCalledWith(expect.objectContaining({ signal: undefined }));
|
||||
});
|
||||
|
||||
it("uses fallback identifier from id+description when no title", async () => {
|
||||
@@ -89,7 +89,7 @@ describe("NtfyNotificationProvider", () => {
|
||||
event: "failed" as any,
|
||||
});
|
||||
|
||||
expect(mocks.sendNtfyNotification).toHaveBeenCalledWith(
|
||||
expect(mocks.sendNtfyNotificationWithResult).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ message: expect.stringContaining('Task "FN-1: desc"') }),
|
||||
);
|
||||
});
|
||||
@@ -137,7 +137,7 @@ describe("NtfyNotificationProvider", () => {
|
||||
},
|
||||
});
|
||||
|
||||
expect(mocks.sendNtfyNotification).toHaveBeenCalledWith(
|
||||
expect(mocks.sendNtfyNotificationWithResult).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
title: "Re: reply preview",
|
||||
}),
|
||||
|
||||
@@ -115,6 +115,7 @@ export {
|
||||
buildNtfyClickUrl,
|
||||
sendNtfyNotification,
|
||||
formatTaskIdentifier,
|
||||
getActiveNotificationService,
|
||||
type NtfyNotifierOptions,
|
||||
type NtfyNotificationPriority,
|
||||
type NtfyNotificationConfigInput,
|
||||
|
||||
@@ -41,6 +41,7 @@ export class NotificationService {
|
||||
private notificationsEnabled = false;
|
||||
private ntfyProvider?: NtfyNotificationProvider;
|
||||
private webhookProvider?: WebhookNotificationProvider;
|
||||
private refreshInFlight: Promise<void> | null = null;
|
||||
|
||||
constructor(
|
||||
private readonly store: NotificationServiceStore,
|
||||
@@ -236,8 +237,19 @@ export class NotificationService {
|
||||
}
|
||||
|
||||
private handleMessageSent = (message: Message): void => {
|
||||
void this.handleMessageSentAsync(message);
|
||||
};
|
||||
|
||||
private async handleMessageSentAsync(message: Message): Promise<void> {
|
||||
schedulerLog.log(
|
||||
`NotificationService.handleMessageSent messageId=${message.id} type=${message.type} notificationsEnabled=${String(this.notificationsEnabled)} hasNtfyProvider=${String(Boolean(this.ntfyProvider))}`,
|
||||
);
|
||||
|
||||
if (!this.notificationsEnabled) {
|
||||
return;
|
||||
await this.refreshNotificationState("message:sent");
|
||||
if (!this.notificationsEnabled) {
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
let eventType: NotificationEvent;
|
||||
@@ -270,7 +282,11 @@ export class NotificationService {
|
||||
preview,
|
||||
},
|
||||
});
|
||||
};
|
||||
|
||||
schedulerLog.log(
|
||||
`NotificationService.handleMessageSent scheduled eventType=${eventType} messageId=${message.id}`,
|
||||
);
|
||||
}
|
||||
|
||||
private setNotificationsEnabledFromSettings(settings: Settings): void {
|
||||
this.notificationsEnabled = Boolean(
|
||||
@@ -281,13 +297,37 @@ export class NotificationService {
|
||||
|
||||
async dispatch(eventType: NotificationEvent, payload: NotificationPayload): Promise<void> {
|
||||
if (!this.notificationsEnabled) {
|
||||
return;
|
||||
await this.refreshNotificationState("manual-dispatch");
|
||||
if (!this.notificationsEnabled) {
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
const dedupTaskId = payload.taskId ?? "global";
|
||||
this.maybeNotify(dedupTaskId, eventType, payload);
|
||||
}
|
||||
|
||||
private async refreshNotificationState(reason: string): Promise<void> {
|
||||
if (this.refreshInFlight) {
|
||||
await this.refreshInFlight;
|
||||
return;
|
||||
}
|
||||
|
||||
this.refreshInFlight = (async () => {
|
||||
const settings = await this.store.getSettings();
|
||||
this.setNotificationsEnabledFromSettings(settings);
|
||||
await this.syncNtfyProvider(settings);
|
||||
await this.syncWebhookProvider(settings);
|
||||
schedulerLog.log(`NotificationService refreshed notification state reason=${reason} enabled=${String(this.notificationsEnabled)}`);
|
||||
})();
|
||||
|
||||
try {
|
||||
await this.refreshInFlight;
|
||||
} finally {
|
||||
this.refreshInFlight = null;
|
||||
}
|
||||
}
|
||||
|
||||
private createTaskPayload(task: Task, event: NotificationEvent): NotificationPayload {
|
||||
return {
|
||||
taskId: task.id,
|
||||
@@ -300,10 +340,12 @@ export class NotificationService {
|
||||
private maybeNotify(taskId: string, eventType: NotificationEvent, payload: NotificationPayload): void {
|
||||
const key = `${taskId}:${eventType}`;
|
||||
if (this.notifiedEvents.has(key)) {
|
||||
schedulerLog.log(`NotificationService.maybeNotify suppressed duplicate key=${key}`);
|
||||
return;
|
||||
}
|
||||
|
||||
this.notifiedEvents.add(key);
|
||||
schedulerLog.log(`NotificationService.maybeNotify dispatching key=${key}`);
|
||||
this.dispatcher.dispatch(eventType, payload).catch(() => {
|
||||
// best effort dispatch
|
||||
});
|
||||
|
||||
@@ -11,8 +11,9 @@ import {
|
||||
buildNtfyClickUrl,
|
||||
formatTaskIdentifier,
|
||||
resolveNtfyEvents,
|
||||
sendNtfyNotification,
|
||||
sendNtfyNotificationWithResult,
|
||||
} from "../notifier.js";
|
||||
import { schedulerLog } from "../logger.js";
|
||||
|
||||
export interface NtfyProviderConfig {
|
||||
/** ntfy topic name */
|
||||
@@ -75,11 +76,16 @@ export class NtfyNotificationProvider implements NotificationProvider {
|
||||
|
||||
isEventSupported(event: NotificationEvent): boolean {
|
||||
if (!SUPPORTED_EVENTS.has(event as SupportedNtfyEvent)) {
|
||||
schedulerLog.log(`NtfyNotificationProvider event filtered unsupported event=${event}`);
|
||||
return false;
|
||||
}
|
||||
|
||||
const enabledEvents = this.config?.events ?? [...DEFAULT_NTFY_EVENTS];
|
||||
return enabledEvents.includes(event as NtfyNotificationEvent);
|
||||
const allowed = enabledEvents.includes(event as NtfyNotificationEvent);
|
||||
schedulerLog.log(
|
||||
`NtfyNotificationProvider allowlist event=${event} decision=${allowed ? "allowed" : "filtered-by-event"}`,
|
||||
);
|
||||
return allowed;
|
||||
}
|
||||
|
||||
async sendNotification(
|
||||
@@ -173,7 +179,20 @@ export class NtfyNotificationProvider implements NotificationProvider {
|
||||
};
|
||||
|
||||
const content = contentByEvent[event as SupportedNtfyEvent];
|
||||
await sendNtfyNotification({
|
||||
const resolvedBaseUrl = this.config.ntfyBaseUrl?.trim() || "https://ntfy.sh";
|
||||
const host = (() => {
|
||||
try {
|
||||
return new URL(resolvedBaseUrl).host;
|
||||
} catch {
|
||||
return "invalid-host";
|
||||
}
|
||||
})();
|
||||
|
||||
schedulerLog.log(
|
||||
`NtfyNotificationProvider send event=${event} host=${host} topic=${this.config.topic}`,
|
||||
);
|
||||
|
||||
const response = await sendNtfyNotificationWithResult({
|
||||
ntfyBaseUrl: this.config.ntfyBaseUrl,
|
||||
topic: this.config.topic,
|
||||
title: content.title,
|
||||
@@ -183,6 +202,14 @@ export class NtfyNotificationProvider implements NotificationProvider {
|
||||
signal: this.abortController?.signal,
|
||||
});
|
||||
|
||||
return { success: true, providerId: this.getProviderId() };
|
||||
schedulerLog.log(
|
||||
`NtfyNotificationProvider delivery event=${event} status=${response?.status ?? "error"} ok=${String(response?.ok ?? false)}`,
|
||||
);
|
||||
|
||||
return {
|
||||
success: Boolean(response?.ok),
|
||||
providerId: this.getProviderId(),
|
||||
...(response?.ok ? {} : { error: response ? `${response.status} ${response.statusText}` : "request failed" }),
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
@@ -136,7 +136,7 @@ export function buildNtfyClickUrl(options: {
|
||||
* Send a notification to ntfy.
|
||||
* Errors are logged and swallowed so callers can treat delivery as best-effort.
|
||||
*/
|
||||
export async function sendNtfyNotification({
|
||||
export async function sendNtfyNotificationWithResult({
|
||||
ntfyBaseUrl,
|
||||
topic,
|
||||
title,
|
||||
@@ -144,7 +144,7 @@ export async function sendNtfyNotification({
|
||||
priority = "default",
|
||||
clickUrl,
|
||||
signal,
|
||||
}: SendNtfyNotificationInput): Promise<void> {
|
||||
}: SendNtfyNotificationInput): Promise<{ ok: boolean; status: number; statusText: string } | null> {
|
||||
try {
|
||||
const headers: Record<string, string> = {
|
||||
Title: title,
|
||||
@@ -167,14 +167,25 @@ export async function sendNtfyNotification({
|
||||
if (!response.ok) {
|
||||
schedulerLog.log(`Ntfy notification failed: ${response.status} ${response.statusText}`);
|
||||
}
|
||||
|
||||
return {
|
||||
ok: response.ok,
|
||||
status: response.status,
|
||||
statusText: response.statusText,
|
||||
};
|
||||
} catch (err) {
|
||||
if (err instanceof Error && err.name === "AbortError") {
|
||||
return;
|
||||
return null;
|
||||
}
|
||||
schedulerLog.log(`Failed to send ntfy notification: ${err}`);
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
export async function sendNtfyNotification(input: SendNtfyNotificationInput): Promise<void> {
|
||||
await sendNtfyNotificationWithResult(input);
|
||||
}
|
||||
|
||||
/**
|
||||
* NtfyNotifier is a backward-compatible wrapper around NotificationService.
|
||||
* It keeps legacy APIs (getConfig, notifyGridlock) while delegating task event
|
||||
@@ -182,6 +193,10 @@ export async function sendNtfyNotification({
|
||||
*/
|
||||
let activeNotificationService: NotificationService | undefined;
|
||||
|
||||
export function getActiveNotificationService(): NotificationService | undefined {
|
||||
return activeNotificationService;
|
||||
}
|
||||
|
||||
export interface FallbackNotificationInput {
|
||||
primaryModel: string;
|
||||
fallbackModel: string;
|
||||
|
||||
Reference in New Issue
Block a user