fix(notifier-desktop): use terminal-notifier on macOS for click-to-open support
On macOS, when terminal-notifier is installed (brew install terminal-notifier), desktop notifications now open the dashboard URL when clicked instead of opening Script Editor. Falls back to osascript when terminal-notifier is not available. New config option `dashboardUrl` controls the click-through URL: notifiers.desktop.dashboardUrl: "http://localhost:8080" Refs #1579
This commit is contained in:
parent
9bfd7656bb
commit
3bea2e9c9e
|
|
@ -4,6 +4,7 @@ import type { OrchestratorEvent, NotifyAction } from "@aoagents/ao-core";
|
|||
// Mock node:child_process
|
||||
vi.mock("node:child_process", () => ({
|
||||
execFile: vi.fn(),
|
||||
execFileSync: vi.fn(),
|
||||
}));
|
||||
|
||||
// Mock node:os
|
||||
|
|
@ -11,11 +12,12 @@ vi.mock("node:os", () => ({
|
|||
platform: vi.fn(() => "darwin"),
|
||||
}));
|
||||
|
||||
import { execFile } from "node:child_process";
|
||||
import { execFile, execFileSync } from "node:child_process";
|
||||
import { platform } from "node:os";
|
||||
import { manifest, create, escapeAppleScript } from "./index.js";
|
||||
|
||||
const mockExecFile = execFile as unknown as Mock;
|
||||
const mockExecFileSync = execFileSync as unknown as Mock;
|
||||
const mockPlatform = platform as unknown as Mock;
|
||||
|
||||
function makeEvent(overrides: Partial<OrchestratorEvent> = {}): OrchestratorEvent {
|
||||
|
|
@ -36,6 +38,10 @@ describe("notifier-desktop", () => {
|
|||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
mockPlatform.mockReturnValue("darwin");
|
||||
// Default: terminal-notifier not available (osascript fallback)
|
||||
mockExecFileSync.mockImplementation(() => {
|
||||
throw new Error("not found");
|
||||
});
|
||||
mockExecFile.mockImplementation(
|
||||
(_cmd: string, _args: string[], cb: (err: Error | null) => void) => {
|
||||
cb(null);
|
||||
|
|
@ -267,4 +273,99 @@ describe("notifier-desktop", () => {
|
|||
await expect(notifier.notify(makeEvent())).rejects.toThrow("osascript not found");
|
||||
});
|
||||
});
|
||||
|
||||
describe("terminal-notifier on macOS", () => {
|
||||
beforeEach(() => {
|
||||
// terminal-notifier is available
|
||||
mockExecFileSync.mockReturnValue(Buffer.from("/usr/local/bin/terminal-notifier\n"));
|
||||
});
|
||||
|
||||
it("uses terminal-notifier when available", async () => {
|
||||
const notifier = create();
|
||||
await notifier.notify(makeEvent());
|
||||
|
||||
expect(mockExecFile).toHaveBeenCalledOnce();
|
||||
expect(mockExecFile.mock.calls[0][0]).toBe("terminal-notifier");
|
||||
});
|
||||
|
||||
it("passes -title and -message args", async () => {
|
||||
const notifier = create();
|
||||
await notifier.notify(makeEvent({ sessionId: "s-1", message: "hello" }));
|
||||
|
||||
const args = mockExecFile.mock.calls[0][1] as string[];
|
||||
expect(args).toContain("-title");
|
||||
expect(args).toContain("-message");
|
||||
expect(args[args.indexOf("-message") + 1]).toBe("hello");
|
||||
});
|
||||
|
||||
it("passes -open with dashboardUrl when configured", async () => {
|
||||
const notifier = create({ dashboardUrl: "http://localhost:8080" });
|
||||
await notifier.notify(makeEvent());
|
||||
|
||||
const args = mockExecFile.mock.calls[0][1] as string[];
|
||||
expect(args).toContain("-open");
|
||||
expect(args[args.indexOf("-open") + 1]).toBe("http://localhost:8080");
|
||||
});
|
||||
|
||||
it("does not pass -open when dashboardUrl is not configured", async () => {
|
||||
const notifier = create();
|
||||
await notifier.notify(makeEvent());
|
||||
|
||||
const args = mockExecFile.mock.calls[0][1] as string[];
|
||||
expect(args).not.toContain("-open");
|
||||
});
|
||||
|
||||
it("passes -sound default for urgent notifications", async () => {
|
||||
const notifier = create();
|
||||
await notifier.notify(makeEvent({ priority: "urgent" }));
|
||||
|
||||
const args = mockExecFile.mock.calls[0][1] as string[];
|
||||
expect(args).toContain("-sound");
|
||||
expect(args[args.indexOf("-sound") + 1]).toBe("default");
|
||||
});
|
||||
|
||||
it("does not pass -sound for non-urgent notifications", async () => {
|
||||
const notifier = create();
|
||||
await notifier.notify(makeEvent({ priority: "info" }));
|
||||
|
||||
const args = mockExecFile.mock.calls[0][1] as string[];
|
||||
expect(args).not.toContain("-sound");
|
||||
});
|
||||
|
||||
it("respects sound=false config", async () => {
|
||||
const notifier = create({ sound: false });
|
||||
await notifier.notify(makeEvent({ priority: "urgent" }));
|
||||
|
||||
const args = mockExecFile.mock.calls[0][1] as string[];
|
||||
expect(args).not.toContain("-sound");
|
||||
});
|
||||
|
||||
it("falls back to osascript when terminal-notifier is not found", async () => {
|
||||
mockExecFileSync.mockImplementation(() => {
|
||||
throw new Error("not found");
|
||||
});
|
||||
const notifier = create();
|
||||
await notifier.notify(makeEvent());
|
||||
|
||||
expect(mockExecFile.mock.calls[0][0]).toBe("osascript");
|
||||
});
|
||||
|
||||
it("does not use terminal-notifier on Linux", async () => {
|
||||
mockPlatform.mockReturnValue("linux");
|
||||
const notifier = create();
|
||||
await notifier.notify(makeEvent());
|
||||
|
||||
expect(mockExecFile.mock.calls[0][0]).toBe("notify-send");
|
||||
});
|
||||
|
||||
it("uses terminal-notifier for notifyWithActions too", async () => {
|
||||
const notifier = create({ dashboardUrl: "http://localhost:3000" });
|
||||
const actions: NotifyAction[] = [{ label: "View", url: "https://example.com" }];
|
||||
await notifier.notifyWithActions!(makeEvent(), actions);
|
||||
|
||||
expect(mockExecFile.mock.calls[0][0]).toBe("terminal-notifier");
|
||||
const args = mockExecFile.mock.calls[0][1] as string[];
|
||||
expect(args).toContain("-open");
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
import { execFile } from "node:child_process";
|
||||
import { execFile, execFileSync } from "node:child_process";
|
||||
import { platform } from "node:os";
|
||||
import {
|
||||
escapeAppleScript,
|
||||
|
|
@ -44,31 +44,60 @@ function formatActionsMessage(event: OrchestratorEvent, actions: NotifyAction[])
|
|||
return `${event.message}\n\nActions: ${actionLabels}`;
|
||||
}
|
||||
|
||||
/** Check once at create() time whether terminal-notifier is available. */
|
||||
function detectTerminalNotifier(): boolean {
|
||||
try {
|
||||
execFileSync("which", ["terminal-notifier"], { stdio: "ignore" });
|
||||
return true;
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Send a desktop notification using osascript (macOS) or notify-send (Linux).
|
||||
* Falls back gracefully if neither is available.
|
||||
* Send a desktop notification using terminal-notifier / osascript (macOS) or
|
||||
* notify-send (Linux). Falls back gracefully if neither is available.
|
||||
*
|
||||
* Note: Desktop notifications do not support click-through URLs natively.
|
||||
* On macOS, osascript's `display notification` lacks URL support.
|
||||
* Consider `terminal-notifier` for click-to-open if needed in the future.
|
||||
* On macOS, when `terminal-notifier` is installed, notifications support
|
||||
* click-to-open: clicking the banner opens `openUrl` in the default browser.
|
||||
* Without it, the osascript fallback is used (no click-through).
|
||||
*/
|
||||
function sendNotification(
|
||||
title: string,
|
||||
message: string,
|
||||
options: { sound: boolean; isUrgent: boolean },
|
||||
options: {
|
||||
sound: boolean;
|
||||
isUrgent: boolean;
|
||||
useTerminalNotifier: boolean;
|
||||
openUrl?: string;
|
||||
},
|
||||
): Promise<void> {
|
||||
return new Promise((resolve, reject) => {
|
||||
const os = platform();
|
||||
|
||||
if (os === "darwin") {
|
||||
const safeTitle = escapeAppleScript(title);
|
||||
const safeMessage = escapeAppleScript(message);
|
||||
const soundClause = options.sound ? ' sound name "default"' : "";
|
||||
const script = `display notification "${safeMessage}" with title "${safeTitle}"${soundClause}`;
|
||||
execFile("osascript", ["-e", script], (err) => {
|
||||
if (err) reject(err);
|
||||
else resolve();
|
||||
});
|
||||
if (options.useTerminalNotifier) {
|
||||
const args = ["-title", title, "-message", message];
|
||||
if (options.openUrl) {
|
||||
args.push("-open", options.openUrl);
|
||||
}
|
||||
if (options.sound) {
|
||||
args.push("-sound", "default");
|
||||
}
|
||||
execFile("terminal-notifier", args, (err) => {
|
||||
if (err) reject(err);
|
||||
else resolve();
|
||||
});
|
||||
} else {
|
||||
const safeTitle = escapeAppleScript(title);
|
||||
const safeMessage = escapeAppleScript(message);
|
||||
const soundClause = options.sound ? ' sound name "default"' : "";
|
||||
const script = `display notification "${safeMessage}" with title "${safeTitle}"${soundClause}`;
|
||||
execFile("osascript", ["-e", script], (err) => {
|
||||
if (err) reject(err);
|
||||
else resolve();
|
||||
});
|
||||
}
|
||||
} else if (os === "linux") {
|
||||
// Linux urgency is driven by event priority, not the macOS sound config
|
||||
const args: string[] = [];
|
||||
|
|
@ -89,6 +118,8 @@ function sendNotification(
|
|||
|
||||
export function create(config?: Record<string, unknown>): Notifier {
|
||||
const soundEnabled = typeof config?.sound === "boolean" ? config.sound : true;
|
||||
const dashboardUrl = typeof config?.dashboardUrl === "string" ? config.dashboardUrl : undefined;
|
||||
const hasTerminalNotifier = platform() === "darwin" && detectTerminalNotifier();
|
||||
|
||||
return {
|
||||
name: "desktop",
|
||||
|
|
@ -98,7 +129,12 @@ export function create(config?: Record<string, unknown>): Notifier {
|
|||
const message = formatMessage(event);
|
||||
const sound = shouldPlaySound(event.priority, soundEnabled);
|
||||
const isUrgent = event.priority === "urgent";
|
||||
await sendNotification(title, message, { sound, isUrgent });
|
||||
await sendNotification(title, message, {
|
||||
sound,
|
||||
isUrgent,
|
||||
useTerminalNotifier: hasTerminalNotifier,
|
||||
openUrl: dashboardUrl,
|
||||
});
|
||||
},
|
||||
|
||||
async notifyWithActions(event: OrchestratorEvent, actions: NotifyAction[]): Promise<void> {
|
||||
|
|
@ -108,7 +144,12 @@ export function create(config?: Record<string, unknown>): Notifier {
|
|||
const message = formatActionsMessage(event, actions);
|
||||
const sound = shouldPlaySound(event.priority, soundEnabled);
|
||||
const isUrgent = event.priority === "urgent";
|
||||
await sendNotification(title, message, { sound, isUrgent });
|
||||
await sendNotification(title, message, {
|
||||
sound,
|
||||
isUrgent,
|
||||
useTerminalNotifier: hasTerminalNotifier,
|
||||
openUrl: dashboardUrl,
|
||||
});
|
||||
},
|
||||
};
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue